Skip to content
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

JS: Add generated typings to SQL models #10253

Merged
merged 8 commits into from Sep 23, 2022

Conversation

asgerf
Copy link
Contributor

@asgerf asgerf commented Sep 1, 2022

This refactors most of the SQL and NoSQL models to rely on MaD type-definitions generated from .d.ts files from their respective libraries.

Most of the infrastructure behind this was merged in #10205 and #10206.

Model.json files

Each MaD model lives in a folder containing a model.json and Model.qll file. The .qll file is entirely generated from the .json file and isn't something you'd normally look at.

Some information about the model.json files:

  • It lists the exact library versions used to produce the model.
  • The generatedModel field contains CSV rows entirely generated by madman, which can be regenerated from the rest of the file. It only contains type definitions and type variables (no sources and sinks).
  • The model field contains user-maintained CSV rows to be merged with those from generatedModel. Anything added via the interactive mode in madman gets added here as well (i.e. sinks).
  • The usedTypes field lists package;type pairs that should be considered relevant (i.e. not pruned), since we expect to use from from ModelOutput::getATypeNode
  • The replaceTypeParameters field identifies type parameters that should be seen as an alias for another type. This can help deal with typings that are parameterized in ways that we don't care about.

Comparison to hand-written models

The initial version of the generated models were not a perfect improvement over hand-written models, as both models covered some cases that were missing in the other one (due to incomplete typings). I manually added the missing parts to the model section, or via the replaceTypeParameters feature, to ensure a final model that covers everything.

Previously we'd have roughly one predicate per type we cared about, for example,

  /** Gets a data flow node referring to a MongoDB collection. */
  private API::Node getACollection() {
    exists(API::Node collection |
      collection = getAMongoClientOrDatabase().getMember("collection").getReturn()
    |
      result = collection
      or
      result = collection.getParameter(1).getParameter(0)
    )
    or
    result = API::Node::ofType("mongodb", "Collection")
  }

Now, only the last clause is relevant. Simply doing API::Node::ofType("mongodb", "Collection") should give everything from the above, because the MaD tables contain the information previously hand-written in the bodies of these predicates.

Evaluations

Evaluation 1 and Evaluation 2 cover 200 projects that depend on various SQL/NoSQL libraries, and show in total:

  • 637 new SQL injection sinks
  • 8 new SQL injection alerts

@asgerf asgerf added the JS label Sep 1, 2022
@asgerf asgerf force-pushed the js/type-defs-squashed branch 3 times, most recently from 6ffbbba to fe16a73 Compare Sep 5, 2022
@asgerf asgerf force-pushed the js/type-defs-squashed branch 2 times, most recently from efe1273 to 112abba Compare Sep 5, 2022
@asgerf asgerf force-pushed the js/type-defs-squashed branch 3 times, most recently from 8cb6f54 to bc6741f Compare Sep 19, 2022
@asgerf asgerf force-pushed the js/type-defs-squashed branch from bc6741f to 2fc5961 Compare Sep 20, 2022
@asgerf asgerf marked this pull request as ready for review Sep 20, 2022
@asgerf asgerf requested review from a team as code owners Sep 20, 2022
RasmusWL
RasmusWL previously approved these changes Sep 20, 2022
Copy link
Member

@RasmusWL RasmusWL left a comment

Pyhton/Ruby changes looks fine 😊

@calumgrant calumgrant requested a review from erik-krogh Sep 20, 2022
Copy link
Contributor

@erik-krogh erik-krogh left a comment

This is much more than "just" adding generated typings.
You've greatly improved beyond adding generated typings.

Solid work 💪

A single optional suggestion, and a comment about the test-output.

Copy link
Contributor

@erik-krogh erik-krogh left a comment

👍

@asgerf asgerf merged commit 11ba0f0 into github:main Sep 23, 2022
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants