Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"Poor Logging Practice" page is incomplete #734

Open
Marcono1234 opened this issue Mar 4, 2023 · 3 comments
Open

"Poor Logging Practice" page is incomplete #734

Marcono1234 opened this issue Mar 4, 2023 · 3 comments

Comments

@Marcono1234
Copy link

Marcono1234 commented Mar 4, 2023

The page "Poor Logging Practice" is incomplete and is missing information:

  • "Risk Factors" says "TBD"

  • A lot of sections contain dummy values, e.g. "Examples", "Related Attacks", ...

  • The sections say "good practice" / "poor practice" without ever explaining why it is considered good or bad; this is not very helpful

  • Given that this is an OWASP page, and the URL path even includes .../vulnerabilities/..., the page never properly mentions what the security aspects are. The only vague security related statement it contains is:

    It can also cause log messages accidentally returned to the end users, revealing internal information to attackers.

    When the use of system output streams is jumbled together with the code that uses loggers properly, the result is often a well-kept log that is missing critical information. In addition, using system output streams can also cause log messages accidentally returned to end users, revealing application internal information to attackers.

    (I am not completely sure though what "cause log messages accidentally returned to the end users" is supposed to mean here, is that about log injection (that has its own page already)?)

If there are really security aspects to this, then it would be good if they are more clearly highlighted (especially for non-static final loggers and multiple loggers). Otherwise if this is just general programming advice maybe this should not be part of the OWASP articles?

@kingthorin
Copy link
Contributor

It's a community page, go ahead and update it.

@Marcono1234
Copy link
Author

Thanks for improving the page!

Though as mentioned above, the security / vulnerability aspects of this article are not really clear to me so I don't think I can really improve it in any way. I will unassign myself from this issue again. Sorry that this might not be very helpful, if you want you can close this issue, or maybe repurpose it for discussion regarding what specific security / vulnerability aspects there are.

For what it's worth, this OWASP page was referenced in the comments of a Stack Overflow question regarding loggers being declared static final. But then this whole topic would still be general programming advice and not related to security (and therefore not in scope for OWASP?). And it seems the question whether a logger field should be static final can not be that conclusively answered, see https://www.slf4j.org/faq.html#declared_static.

@Marcono1234 Marcono1234 removed their assignment Mar 4, 2023
@kingthorin
Copy link
Contributor

It’s a community page anyone can contribute to it.

I’ve addressed the most simple bits.

You or anyone else is welcome to add/edit the content further. As with any content in this section of the site.

As this particular page is no more or less special than any other I’m going to close the issue. There are tons that could use improvement/editing, there’s nothing unique here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants