-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Python: Exclude certificate classification for sensitive data queries #17314
Conversation
…t-storage and cleartext-logging queries
c4194a5
to
a8591c7
Compare
def get_password(): | ||
return "password" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@joefarebrother Could you elaborate on the rationale behind this change? The non-Python3 version tests certificate and password writes. Whereas the Python3 test now seems to have switched from testing certificate writes to testing password writes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No significant reason. Would it be best to include tests for both?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My interpretation is that this -py3
test is here to ensure we properly support writing through pathlib.Path
methods for this query, so LGTM 👍
Co-authored-by: Sid Shankar <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Besides minor style NIT, looks good to me 👍
python/ql/lib/semmle/python/security/dataflow/CleartextLoggingCustomizations.qll
Outdated
Show resolved
Hide resolved
python/ql/lib/semmle/python/security/dataflow/CleartextStorageCustomizations.qll
Outdated
Show resolved
Hide resolved
def get_password(): | ||
return "password" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My interpretation is that this -py3
test is here to ensure we properly support writing through pathlib.Path
methods for this query, so LGTM 👍
Co-authored-by: Rasmus Wriedt Larsen <[email protected]>
A certificate, such as an SSL certificate or x509 certificate, often does not contain sensitive data, so the cleartext storage and cleartext logging queries result in false positive alerts when considering them. This PR excludes certificates as sources for these queries.