Skip to content

Conversation

michaelnebel
Copy link
Contributor

@michaelnebel michaelnebel commented Oct 11, 2022

The purpose of this PR is to use data extensions for MaD models instead of inlining them in the Code QL source code for C#.

To that end we

  • Introduce a script that produces data extension (YAML) files for the existing hand-written models. The scripts can be used in the future, if we want to change the generated YAML for our existing hand-written models.
  • Introduce a new model generator scripts that produces a data extension YAML file instead of a Code QL source code file.
  • Introduce the relevant extensible predicates in ExternalFlow.qll.
  • Add hand-written models as data extensions and cleanup the Code QL source code for hand-written MaD models.
  • Add generated models as data extensions for .NET runtime (including negative models) and cleanup the Code QL source for generated and negative models.

Before making any changes the generated .NET runtime models were refreshed as a part of this PR.

@github-actions github-actions bot added the C# label Oct 11, 2022
@michaelnebel michaelnebel force-pushed the csharp/generatedataextensions branch 5 times, most recently from 5e8692d to 734a98b Compare October 13, 2022 14:37
@github-actions
Copy link
Contributor

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

csharp

Generated file changes for csharp

  • Changes to framework-coverage-csharp.rst:
-    System,"``System.*``, ``System``",4,12131,65,7
+    System,"``System.*``, ``System``",4,12142,65,7
-    Totals,,4,12694,397,7
+    Totals,,4,12705,397,7
  • Changes to framework-coverage-csharp.csv:
- System,65,4,12131,,8,8,9,,4,,33,3,1,3,10139,1992
+ System,65,4,12142,,8,8,9,,4,,33,3,1,3,10151,1991

@michaelnebel michaelnebel force-pushed the csharp/generatedataextensions branch from 734a98b to 56bb249 Compare October 14, 2022 08:24
@github-actions
Copy link
Contributor

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

csharp

Generated file changes for csharp

  • Changes to framework-coverage-csharp.rst:
-    System,"``System.*``, ``System``",4,12131,65,7
+    System,"``System.*``, ``System``",4,12142,65,7
-    Totals,,4,12694,397,7
+    Totals,,4,12705,397,7
  • Changes to framework-coverage-csharp.csv:
- System,65,4,12131,,8,8,9,,4,,33,3,1,3,10139,1992
+ System,65,4,12142,,8,8,9,,4,,33,3,1,3,10151,1991

@github-actions
Copy link
Contributor

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

csharp

Generated file changes for csharp

  • Changes to framework-coverage-csharp.rst:
-    System,"``System.*``, ``System``",4,12131,65,7
+    System,"``System.*``, ``System``",4,12142,65,7
-    Totals,,4,12694,397,7
+    Totals,,4,12705,397,7
  • Changes to framework-coverage-csharp.csv:
- System,65,4,12131,,8,8,9,,4,,33,3,1,3,10139,1992
+ System,65,4,12142,,8,8,9,,4,,33,3,1,3,10151,1991

@michaelnebel michaelnebel force-pushed the csharp/generatedataextensions branch from c4872aa to 54bd84c Compare October 14, 2022 13:31
@github-actions
Copy link
Contributor

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

csharp

Generated file changes for csharp

  • Changes to framework-coverage-csharp.rst:
-    System,"``System.*``, ``System``",4,12131,65,7
+    System,"``System.*``, ``System``",4,12142,65,7
-    Totals,,4,12694,397,7
+    Totals,,4,12705,397,7
  • Changes to framework-coverage-csharp.csv:
- System,65,4,12131,,8,8,9,,4,,33,3,1,3,10139,1992
+ System,65,4,12142,,8,8,9,,4,,33,3,1,3,10151,1991

@michaelnebel michaelnebel force-pushed the csharp/generatedataextensions branch from 54bd84c to b3f1979 Compare October 24, 2022 14:49
@github-actions
Copy link
Contributor

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

csharp

Generated file changes for csharp

  • Changes to framework-coverage-csharp.rst:
-    System,"``System.*``, ``System``",4,12131,65,7
+    System,"``System.*``, ``System``",4,12142,65,7
-    Totals,,4,12694,397,7
+    Totals,,4,12705,397,7
  • Changes to framework-coverage-csharp.csv:
- System,65,4,12131,,8,8,9,,4,,33,3,1,3,10139,1992
+ System,65,4,12142,,8,8,9,,4,,33,3,1,3,10151,1991

@github-actions
Copy link
Contributor

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

csharp

Generated file changes for csharp

  • Changes to framework-coverage-csharp.rst:
-    System,"``System.*``, ``System``",4,12131,65,7
+    System,"``System.*``, ``System``",4,12142,65,7
-    Totals,,4,12694,397,7
+    Totals,,4,12705,397,7
  • Changes to framework-coverage-csharp.csv:
- System,65,4,12131,,8,8,9,,4,,33,3,1,3,10139,1992
+ System,65,4,12142,,8,8,9,,4,,33,3,1,3,10151,1991

@github-actions
Copy link
Contributor

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

csharp

Generated file changes for csharp

  • Changes to framework-coverage-csharp.rst:
-    System,"``System.*``, ``System``",4,12131,65,7
+    System,"``System.*``, ``System``",4,12142,65,7
-    Totals,,4,12694,397,7
+    Totals,,4,12705,397,7
  • Changes to framework-coverage-csharp.csv:
- System,65,4,12131,,8,8,9,,4,,33,3,1,3,10139,1992
+ System,65,4,12142,,8,8,9,,4,,33,3,1,3,10151,1991

@github-actions
Copy link
Contributor

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

csharp

Generated file changes for csharp

  • Changes to framework-coverage-csharp.rst:
-    System,"``System.*``, ``System``",4,12131,65,7
+    System,"``System.*``, ``System``",4,12142,65,7
-    Totals,,4,12694,397,7
+    Totals,,4,12705,397,7
  • Changes to framework-coverage-csharp.csv:
- System,65,4,12131,,8,8,9,,4,,33,3,1,3,10139,1992
+ System,65,4,12142,,8,8,9,,4,,33,3,1,3,10151,1991

@github-actions
Copy link
Contributor

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

csharp

Generated file changes for csharp

  • Changes to framework-coverage-csharp.rst:
-    System,"``System.*``, ``System``",4,12131,65,7
+    System,"``System.*``, ``System``",4,12142,65,7
-    Totals,,4,12694,397,7
+    Totals,,4,12705,397,7
  • Changes to framework-coverage-csharp.csv:
- System,65,4,12131,,8,8,9,,4,,33,3,1,3,10139,1992
+ System,65,4,12142,,8,8,9,,4,,33,3,1,3,10151,1991

@michaelnebel michaelnebel force-pushed the csharp/generatedataextensions branch from 8ce33c9 to 516d4e7 Compare October 26, 2022 08:46
@github-actions
Copy link
Contributor

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

csharp

Generated file changes for csharp

  • Changes to framework-coverage-csharp.rst:
-    System,"``System.*``, ``System``",4,12131,65,7
+    System,"``System.*``, ``System``",4,12142,65,7
-    Totals,,4,12694,397,7
+    Totals,,4,12705,397,7
  • Changes to framework-coverage-csharp.csv:
- System,65,4,12131,,8,8,9,,4,,33,3,1,3,10139,1992
+ System,65,4,12142,,8,8,9,,4,,33,3,1,3,10151,1991

@michaelnebel michaelnebel force-pushed the csharp/generatedataextensions branch from 516d4e7 to accdd98 Compare October 26, 2022 08:56
@github-actions
Copy link
Contributor

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

csharp

Generated file changes for csharp

  • Changes to framework-coverage-csharp.rst:
-    System,"``System.*``, ``System``",4,12131,65,7
+    System,"``System.*``, ``System``",4,12142,65,7
-    Totals,,4,12694,397,7
+    Totals,,4,12705,397,7
  • Changes to framework-coverage-csharp.csv:
- System,65,4,12131,,8,8,9,,4,,33,3,1,3,10139,1992
+ System,65,4,12142,,8,8,9,,4,,33,3,1,3,10151,1991

@github-actions
Copy link
Contributor

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

csharp

Generated file changes for csharp

  • Changes to framework-coverage-csharp.rst:
-    System,"``System.*``, ``System``",4,12131,65,7
+    System,"``System.*``, ``System``",4,12142,65,7
-    Totals,,4,12694,397,7
+    Totals,,4,12705,397,7
  • Changes to framework-coverage-csharp.csv:
- System,65,4,12131,,8,8,9,,4,,33,3,1,3,10139,1992
+ System,65,4,12142,,8,8,9,,4,,33,3,1,3,10151,1991

@michaelnebel michaelnebel force-pushed the csharp/generatedataextensions branch from dde65df to 7f773ac Compare October 26, 2022 12:51
@github-actions
Copy link
Contributor

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

csharp

Generated file changes for csharp

  • Changes to framework-coverage-csharp.rst:
-    System,"``System.*``, ``System``",4,12131,65,7
+    System,"``System.*``, ``System``",4,12142,65,7
-    Totals,,4,12694,397,7
+    Totals,,4,12705,397,7
  • Changes to framework-coverage-csharp.csv:
- System,65,4,12131,,8,8,9,,4,,33,3,1,3,10139,1992
+ System,65,4,12142,,8,8,9,,4,,33,3,1,3,10151,1991

@michaelnebel michaelnebel force-pushed the csharp/generatedataextensions branch 2 times, most recently from 10db0c2 to b6f5888 Compare October 26, 2022 14:28
@github-actions
Copy link
Contributor

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

csharp

Generated file changes for csharp

  • Changes to framework-coverage-csharp.rst:
-    System,"``System.*``, ``System``",4,12131,65,7
+    System,"``System.*``, ``System``",4,12142,65,7
-    Totals,,4,12694,397,7
+    Totals,,4,12705,397,7
  • Changes to framework-coverage-csharp.csv:
- System,65,4,12131,,8,8,9,,4,,33,3,1,3,10139,1992
+ System,65,4,12142,,8,8,9,,4,,33,3,1,3,10151,1991

@github-actions
Copy link
Contributor

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

csharp

Generated file changes for csharp

  • Changes to framework-coverage-csharp.rst:
-    System,"``System.*``, ``System``",4,12131,65,7
+    System,"``System.*``, ``System``",4,12142,65,7
-    Totals,,4,12694,397,7
+    Totals,,4,12705,397,7
  • Changes to framework-coverage-csharp.csv:
- System,65,4,12131,,8,8,9,,4,,33,3,1,3,10139,1992
+ System,65,4,12142,,8,8,9,,4,,33,3,1,3,10151,1991

@aeisenberg
Copy link
Contributor

cc: @cklin, In previous runs, the C# Language Tests job failed with OOME almost certainly due to 50k tuples from data extensions. We need to think of some ways of being more memory efficient when we read in the tuples.

@michaelnebel michaelnebel force-pushed the csharp/generatedataextensions branch from 0c9965e to 3c8fb05 Compare November 4, 2022 07:23
@github-actions
Copy link
Contributor

github-actions bot commented Nov 4, 2022

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

csharp

Generated file changes for csharp

  • Changes to framework-coverage-csharp.rst:
-    System,"``System.*``, ``System``",4,12131,65,7
+    System,"``System.*``, ``System``",4,12142,65,7
-    Totals,,4,12694,397,7
+    Totals,,4,12705,397,7
  • Changes to framework-coverage-csharp.csv:
- System,65,4,12131,,8,8,9,,4,,33,3,1,3,10139,1992
+ System,65,4,12142,,8,8,9,,4,,33,3,1,3,10151,1991

@michaelnebel michaelnebel requested a review from hvitved November 4, 2022 07:43
Copy link
Contributor

@hvitved hvitved left a comment

Choose a reason for hiding this comment

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

Approach LGTM, and great work on converting all existing flow summaries from .qll to .yml.

Comment on lines +2220 to +2221
private import semmle.code.csharp.frameworks.system.threading.Tasks
private import semmle.code.csharp.frameworks.system.runtime.CompilerServices
Copy link
Contributor

Choose a reason for hiding this comment

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

This files only had code deleted, so why is this needed now and not before (perhaps it was)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is to ensure the bi-directional import for synthetic fields - this is implicitly handled by the bi-directional import in the Frameworks module in ExternalFlow.qll.
It turns out that the massive gordian knot that is the Frameworks module in ExternalFlow can't just be deleted (this needs to be a follow up, if we want to do that). I have document the this on: https://github.com/github/codeql-csharp-team/issues/298

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added an import for a similar reason in TaintTrackingPrivate.qll

@github-actions
Copy link
Contributor

github-actions bot commented Nov 8, 2022

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

csharp

Generated file changes for csharp

  • Changes to framework-coverage-csharp.rst:
-    System,"``System.*``, ``System``",4,12131,65,7
+    System,"``System.*``, ``System``",4,12142,65,7
-    Totals,,4,12694,397,7
+    Totals,,4,12705,397,7
  • Changes to framework-coverage-csharp.csv:
- System,65,4,12131,,8,8,9,,4,,33,3,1,3,10139,1992
+ System,65,4,12142,,8,8,9,,4,,33,3,1,3,10151,1991

@michaelnebel michaelnebel requested a review from hvitved November 9, 2022 12:26
@michaelnebel michaelnebel marked this pull request as ready for review November 9, 2022 13:55
@michaelnebel michaelnebel requested review from a team as code owners November 9, 2022 13:55
@github-actions
Copy link
Contributor

github-actions bot commented Nov 9, 2022

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

csharp

Generated file changes for csharp

  • Changes to framework-coverage-csharp.rst:
-    System,"``System.*``, ``System``",4,12131,65,7
+    System,"``System.*``, ``System``",4,12142,65,7
-    Totals,,4,12694,397,7
+    Totals,,4,12705,397,7
  • Changes to framework-coverage-csharp.csv:
- System,65,4,12131,,8,8,9,,4,,33,3,1,3,10139,1992
+ System,65,4,12142,,8,8,9,,4,,33,3,1,3,10151,1991

@michaelnebel michaelnebel merged commit 9c6875e into github:main Nov 10, 2022
@michaelnebel michaelnebel deleted the csharp/generatedataextensions branch November 10, 2022 12:08
owen-mc added a commit to owen-mc/codeql that referenced this pull request Nov 29, 2022
owen-mc added a commit to owen-mc/codeql that referenced this pull request Nov 29, 2022
owen-mc added a commit to owen-mc/codeql that referenced this pull request Nov 29, 2022
owen-mc added a commit to owen-mc/codeql that referenced this pull request Nov 29, 2022
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.

3 participants