False Positive Secure Logging S5145

Hi,

Sonar reported S5145 on our log statement, do not log user data:

logger.error(“ABCD [{}] fehlerhaft (Status: [{}], [{}])”, var1, var2, var3);

The recommendation to fix is to replace all patter-breaking charaters (\n \r \t) with “_”.

Not we integrated the owasp secure logging with logback and adjusted our pattern in logback.xml to:

%crlf(%mask) %ex %n

Sonar do not recognie this? How can Sonar detect that the log is fully security escaped?

Thank you!

Hello Michael and welcome!

Thanks for reporting this.
I’m not sure I understood the reason behind the False Positive you are reporting:
are you saying that the method logger.error(“ABCD [{}] fehlerhaft (Status: [{}], [{}])”, var1, var2, var3); is called with arguments that do not come from a user, but still raises an issue for rule S5145? Or are you saying that it does, but that the issue should not be raised because it is sanitized due to the logback pattern?

Thanks for helping clarifying your issue.

Best,
-Christophe

We spotted the same issue.

How should sonarqube be aware of your logback configuration?
So if you know you mitigate the security issue by your logback configuration you simply should disable this rule because a mitigation within the code is not necessary.

Kind regards,
Michael

1 Like

I just wanted to point out the second: the issue should not be raised, because it is sanitized due to the logback pattern and used libraries:

%crlf(%mask) %ex %n

Best,
Michael

Ok, thanks for the clarification and for reporting this case !

I will discuss this internally, and see what we want to do about it.
Note that this is not something easy to take into account automatically, as we’d need to understand the properties to read for each potential logging framework used by a project.

Note that if you are 100% sure that you are not vulnerable to log injections in your project, you could indeed disable the rule for this project at the moment.

Thanks.
Best,
-Chris

Sure, this will not be easy.

May be a solution imho e.g. to force configuration by Java and then it would be easier to regex the params (log pattern) of the configuration method and assert the requirements. This in combination with the recommended log library from owasp. OR I don’t know how sonar works internally… you can also grab die log pattern and use a own attack vector with e.g. backslash r backslash n … and check if it escaped.

Yes, we disabled the rule for the moment. (cc: Michael Düsterhus )

Best,
Michael