Add sensitive information exposure query #126
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| init() { | ||
| /* A sensitive info log sink. */ | ||
|
|
||
| LOG.info("Received: ", Sample.name); // CAP log exposure alert |
Check failure
Code scanning / CodeQL
Insertion of sensitive information into log files
javascript/frameworks/cap/lib/advanced_security/javascript/frameworks/cap/CDL.qll
Outdated
Show resolved
Hide resolved
javascript/frameworks/cap/lib/advanced_security/javascript/frameworks/cap/CDL.qll
Outdated
Show resolved
Hide resolved
javascript/frameworks/cap/lib/advanced_security/javascript/frameworks/cap/CDL.qll
Outdated
Show resolved
Hide resolved
javascript/frameworks/cap/src/sensitive-exposure/SensitiveExposure.md
Outdated
Show resolved
Hide resolved
javascript/frameworks/cap/src/sensitive-exposure/SensitiveExposure.md
Outdated
Show resolved
Hide resolved
jeongsoolee09
requested changes
Jul 22, 2024
Contributor
jeongsoolee09
left a comment
jeongsoolee09
left a comment
There was a problem hiding this comment.
I left some thoughts on improving the structure of the query.
mbaluda
reviewed
Jul 31, 2024
|
|
||
| class SensitiveExposureSource extends DataFlow::Node { | ||
| SensitiveExposureSource() { | ||
| exists(PropRead p, SensitiveAnnotatedElement c | |
Contributor
There was a problem hiding this comment.
I see a problem here as you are only comparing on the property name (name in the test vs Sample.name)
There can be multiple entities with the same names as well as multiple applications in the same repo...
I think you can use getCdsDeclaration()
mbaluda
reviewed
Aug 6, 2024
| * @problem.severity warning | ||
| * @security-severity 7.5 | ||
| * @precision medium | ||
| * @id javascript/sensitive-log |
Contributor
There was a problem hiding this comment.
We use the js language code: @id js/sensitive-log
https://github.com/github/codeql/blob/main/docs/query-metadata-style-guide.md#query-id-id
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
currently this query only covers the case of a
PropRead(ie entity field) being the exact match for the annotated sensitive element. Technically also the entire entity (see entity level labels) can be labelled as@PersonalDatabut I am considering to define the first iteration of this with the heuristic that fields in that case are still what also get annotated additionally and exposed.happy to reconsider based on other's thoughts! :)