Introduce shared taint tracking library by jketema · Pull Request #13881 · github/codeql

jketema · 2023-08-03T13:50:12Z

No description provided.

aschackmull · 2023-08-04T09:27:09Z

A few stylistic comments, otherwise mostly LGTM.

+ private import DataFlowMake<DataFlowLang> as DataFlow
+ private import MakeImpl<DataFlowLang> as DataFlowInternal
+
+ private module AddTaintDefaults<DataFlowInternal::FullStateConfigSig Config> implements


+ * Constructs a global taint tracking computation.
+ */
+ module Global<DataFlow::ConfigSig Config> implements DataFlow::GlobalFlowSig {
+ private module Config0 implements DataFlowInternal::FullStateConfigSig {


+ import Config
+ }
+
+ private module C implements DataFlowInternal::FullStateConfigSig {


+ * Constructs a global taint tracking computation using flow state.
+ */
+ module GlobalWithState<DataFlow::StateConfigSig Config> implements DataFlow::GlobalFlowSig {
+ private module Config0 implements DataFlowInternal::FullStateConfigSig {


+ import Config
+ }
+
+ private module C implements DataFlowInternal::FullStateConfigSig {


aschackmull · 2023-08-04T09:58:41Z

LGTM now. Although, preferably we'll switch all languages in one go, so we don't have a mix of shared lib usage and parameterised modules.

jketema · 2023-08-04T10:00:36Z

LGTM now. Although, preferably we'll switch all languages in one go, so we don't have a mix of shared lib usage and parameterised modules.

That is the plan (as part of this PR). I only did C++ first, because I wanted to know if the approach was good.

aschackmull · 2023-08-04T11:42:37Z

Beware that you have an incoming merge conflict in #13851

jketema · 2023-08-04T16:06:58Z

Beware that you have an incoming merge conflict in #13851

I'll rebase this PR and then kick off all the DCAs.

…efactoring

jketema · 2023-08-07T13:25:11Z

I sightly refactored this to follow #13901

jketema · 2023-08-07T13:47:07Z

@aschackmullI'm slightly confused by the DCA results. I see OOMs in Java (which seem to be sort of expected), C# and C++. I tried the C++ and C# OOM'ing projects on larger runners to see if there are any suspicious tuple counts stage timings. I see slightly slower stage timings, in a few cases but nothing really jumps out to me.

I re-ran the DCA experiments (which was before the OOM-related revert we did yesterday). All DCA results look uneventful to me, except for the Java ones which show a large number of OOMs. Let me know if I should re-run those with the OOM-related revert.

hvitved

LGTM 🎉

RasmusWL

👍 for Python

github-actionsBot added the C++ label Aug 3, 2023

github-advanced-securityAI found potential problems Aug 3, 2023
View reviewed changes

aschackmull reviewed Aug 4, 2023
View reviewed changes

Comment threadshared/dataflow/codeql/dataflow/TaintTracking.qll Outdated

aschackmull reviewed Aug 4, 2023
View reviewed changes

Comment threadshared/dataflow/codeql/dataflow/TaintTracking.qll Outdated

aschackmull reviewed Aug 4, 2023
View reviewed changes

Comment threadshared/dataflow/codeql/dataflow/TaintTracking.qll Outdated

github-advanced-securityAI found potential problems Aug 4, 2023
View reviewed changes

github-actionsBot added C# documentation Java Python Go Ruby Swift labels Aug 4, 2023

jketema added 5 commits August 4, 2023 22:51

Introduce shared taint tracking library
bdd64ce

Address review comments
7ba2f7a

Add missing QLDoc
20b7925

Update all languages to use the shared taint-tracking library
747cd17

Add change note
c4a65e5

jketema force-pushed the shared-taint-tracking branch from 9f12a4e to c4a65e5Compare August 4, 2023 20:53

jketema added 2 commits August 7, 2023 15:22

Merge branch 'main' into shared-taint-tracking
5d2984b

Refactor the traint-tracking library to follow the dataflow library r…
8b6a798
…efactoring

Merge branch 'main' into shared-taint-tracking
33e8310

jketema marked this pull request as ready for review August 18, 2023 08:40

jketema requested review from a team as code owners August 18, 2023 08:40

hvitved previously approved these changes Aug 21, 2023
View reviewed changes

aschackmull reviewed Aug 21, 2023
View reviewed changes

Comment threadjava/ql/lib/semmle/code/java/dataflow/TaintTracking.qll

Java: Delete copy of shared taint tracking library
a2bb7de

jketema dismissed hvitved’s stale review via a2bb7deAugust 21, 2023 08:33

aschackmull approved these changes Aug 21, 2023
View reviewed changes

MathiasVP approved these changes Aug 21, 2023
View reviewed changes

RasmusWL approved these changes Aug 21, 2023
View reviewed changes

jketema merged commit 2d0f73d into github:mainAug 21, 2023

jketema deleted the shared-taint-tracking branch August 21, 2023 10:45

Uh oh!

Conversation

jketema commented Aug 3, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aschackmull commented Aug 4, 2023

Uh oh!

Check warning

Check warning

Check warning

Check warning

Check warning

aschackmull commented Aug 4, 2023• edited by jketema Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jketema commented Aug 4, 2023

Uh oh!

aschackmull commented Aug 4, 2023

Uh oh!

jketema commented Aug 4, 2023

Uh oh!

jketema commented Aug 7, 2023

Uh oh!

jketema commented Aug 7, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hvitved left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

RasmusWL left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

aschackmull commented Aug 4, 2023•
edited by jketema
Loading

jketema commented Aug 7, 2023•
edited
Loading