Skip to content

Python: Add taint-sinks meta query #11480

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

Merged
merged 2 commits into from
Dec 1, 2022
Merged

Conversation

RasmusWL
Copy link
Member

No description provided.

@RasmusWL RasmusWL requested a review from a team as a code owner November 29, 2022 14:12
Copy link
Contributor

@tausbn tausbn left a comment

Choose a reason for hiding this comment

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

Seems reasonable to me. 👍

@RasmusWL RasmusWL added the no-change-note-required This PR does not need a change note label Nov 29, 2022
DataFlow::Node relevantTaintSink(string kind) {
not result.getLocation().getFile() instanceof IgnoredFile and
(
kind = "CleartextLogging" and result instanceof CleartextLogging::Sink
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI: in JS I deliberately omitted logging-related sinks in this query, because it ends up dominating the number of sinks.

Copy link
Member Author

Choose a reason for hiding this comment

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

I see two use-cases for this query:

  1. manually running it on a DB to see what interesting sinks have been found
  2. checking differences in sinks found from changes in a PR

For (1) it sounds very reasonable to exclude things that are not super interesting, like the cleartext-logging stuff, but for (2), which is the aim of this work, I would like to keep them.

Do you agree?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, sounds very reasonable.

Comment on lines +81 to +82
or
kind = "Xxe" and result instanceof Xxe::Sink
Copy link
Contributor

@yoff yoff Nov 30, 2022

Choose a reason for hiding this comment

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

I wonder if there is a way of writing this that is easier to maintain. Something like

kind.(TaintTracking::Configuration).isSink(result)

(horribly abusing that Configurations are still strings).

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess we could write it this way, but I personally prefer the current approach -- especially since it will also work once configurations are no longer strings.

@RasmusWL RasmusWL merged commit e7264fb into github:main Dec 1, 2022
@RasmusWL RasmusWL deleted the sink-meta-query branch December 1, 2022 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-change-note-required This PR does not need a change note Python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants