Java: 17052 Second try: do not expose error message #17075
Conversation
java/ql/src/Security/CWE/CWE-209/SensitiveDataExposureThroughErrorMessage.ql
Show resolved
Hide resolved
…es are split out of `java/stack-trace-exposure` into its own alert `java/error-message-exposure` because this is a better fit.
RobbingDaHood
force-pushed
the
17052-second-try-do-not-expose-error-message
branch
from
9849d31
to
1c1ba77
Compare
java/ql/src/change-notes/2024-07-25-java-error-message-exposure.md
Outdated
Show resolved
Hide resolved
java/ql/src/Security/CWE/CWE-209/SensitiveDataExposureThroughErrorMessage.ql
Outdated
Show resolved
Hide resolved
java/ql/src/Security/CWE/CWE-209/SensitiveDataExposureThroughErrorMessage.qhelp
Outdated
Show resolved
Hide resolved
java/ql/src/Security/CWE/CWE-209/SensitiveDataExposureThroughErrorMessage.java
Outdated
Show resolved
Hide resolved
| * @precision high | ||
| * @id java/error-message-exposure | ||
| * @tags security | ||
| * external/cwe/cwe-209 |
There was a problem hiding this comment.
Perhaps this query should also be tagged with cwe-497?
There was a problem hiding this comment.
Sounds like https://cwe.mitre.org/data/definitions/497.html is broader than https://cwe.mitre.org/data/definitions/209.html and will in most cases also be present: I cant think of a case where https://cwe.mitre.org/data/definitions/209.html is relevant where https://cwe.mitre.org/data/definitions/497.html is not.
In the end I do not know who can make this decision, but I would not mind adding it.
Co-authored-by: Anders Schack-Mulligen <[email protected]>
java/ql/src/Security/CWE/CWE-209/SensitiveDataExposureThroughErrorMessage.ql
Show resolved
Hide resolved
|
|
||
| from Expr externalExpr, Expr errorInformation | ||
| where | ||
| getMessageFlowsExternally(DataFlow::exprNode(externalExpr), DataFlow::exprNode(errorInformation)) |
There was a problem hiding this comment.
The current changes does not include checking if the error message is logged or printed out: only that it is exposed in HTTP bodies.
If expanding this query with that check have interest then i can implement this: But likely in another issue and PR: Just to get this PR closed.
| * @precision high | ||
| * @id java/error-message-exposure | ||
| * @tags security | ||
| * external/cwe/cwe-209 |
There was a problem hiding this comment.
Sounds like https://cwe.mitre.org/data/definitions/497.html is broader than https://cwe.mitre.org/data/definitions/209.html and will in most cases also be present: I cant think of a case where https://cwe.mitre.org/data/definitions/209.html is relevant where https://cwe.mitre.org/data/definitions/497.html is not.
In the end I do not know who can make this decision, but I would not mind adding it.
First try were here: #17066
Main reason for this new try were this comment: #17066 (comment)
So now I will create this new PR and close the old one. I also implemented all the changes suggested from the old PR too.
The below is a copy paste from the first PR description:
The problem
Trying to fix: #17052
And the possible mentioned duplicate of: #16867 as mentioned here: #17052 (comment)
The fix
The fix is also described a bit here: #17052 (comment)
This is my first pull request and I did try to read the documentation but likely missed some details: Hope you will point out any error so I can improve.
Test
I did perform the tests in the two test folders that I changed.
I did try to start the "full test" but of all the java tests, but that crashed my machine. So I do not know if I need to perform any other test?