Skip to content

JS: sharpen js/clear-text-logging (ODASA-7485)#520

Merged
semmle-qlci merged 1 commit into
masterfrom
unknown repository
Nov 23, 2018
Merged

JS: sharpen js/clear-text-logging (ODASA-7485)#520
semmle-qlci merged 1 commit into
masterfrom
unknown repository

Conversation

@ghost

@ghost ghost commented Nov 22, 2018

Copy link
Copy Markdown

This PR sharpens js/clear-text-logging by not propagating through property reads, except if the read leads directly to a source.

As may be seen by the branch name, I first anticipated to implement this with flow labels. But in the end, it seemed simpler and safer to just add a single edge barrier.

The evaluation shows that performance is practically unchanged.

@ghost ghost added the JS label Nov 22, 2018
@ghost ghost self-requested a review as a code owner November 22, 2018 09:59
Comment thread javascript/ql/src/semmle/javascript/security/dataflow/CleartextLogging.qll Outdated
@ghost

ghost commented Nov 22, 2018

Copy link
Copy Markdown
Author

Ahem 😊

This is the one line that needs to be changed: b780f82#diff-05bc437cd4f51a47cff43223f844d989L55.

The character of TaintTracking::StringConcatenationTaintStep was way more permissive than expected, and included all AdditionalTaintSteps into a non-taint configuration...

@asger-semmle asger-semmle left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@semmle-qlci semmle-qlci merged commit 04c2b23 into github:master Nov 23, 2018
cklin pushed a commit that referenced this pull request May 23, 2022
Add a new diagnostics file class and use it for errors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants