Skip to content

Conversation

asgerf
Copy link
Contributor

@asgerf asgerf commented Mar 29, 2021

Adds some missing features I noticed while investigating data flows through some projects:

  • Add lodash-es to the lodash model
  • Step through babel.transform
  • Propagate React components through redux-form
  • Add more RouteMatch objects to the react-router model
  • Add react-native-base64 to the base64 library
  • Model o[o.length] = y as a taint step as it does the same as o.push(y)

Evaluations: (internal links)

  • Default slugs shows ok performance.
  • The changes were included in an old Redux evaluation and were responsible for the new code injection and request forgery results.

@asgerf asgerf added JS JS:changes-sources-or-sinks Changes taint sources/sinks for the JS analysis labels Mar 29, 2021
@asgerf asgerf requested a review from a team as a code owner March 29, 2021 10:22
Copy link
Contributor

@esbena esbena left a comment

Choose a reason for hiding this comment

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

LGTM, with one concern about an async api. (I think it is fine to bundle these unrelated changes into a single PR)

exists(DataFlow::CallNode call |
call =
API::moduleImport(["@babel/standalone", "@babel/core"])
.getMember(["transform", "transformSync"])
Copy link
Contributor

Choose a reason for hiding this comment

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

is .transform async? Won't that make the succ turn up as a parameter to a callback?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah you're right. I thought transform would return a promise but after a closer look it just becomes synchronous if you don't give it a callback. There's also transformAsync which does return a promise.

Since neither transformSync nor transformAsync accept a third parameter I just added an extra case for the callback parameter.

@codeql-ci codeql-ci merged commit e8d7925 into github:main Mar 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation JS:changes-sources-or-sinks Changes taint sources/sinks for the JS analysis JS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants