Skip to content

C#: Start using CSV based flow models #6148

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 14 commits into from
Jun 30, 2021

Conversation

tamasvajk
Copy link
Contributor

@tamasvajk tamasvajk commented Jun 23, 2021

This PR

  • migrates some sources, steps, and sinks to the new CSV format;
  • adjusts the coverage report generators and workflows to cover C# too;
  • modifies the RST generator to allow specifying multiple sink ids per CWE number.

The workflows have been tested here:

@github-actions github-actions bot added the C# label Jun 23, 2021
@tamasvajk tamasvajk force-pushed the feature/try-csv-source-models branch from 71a4822 to 9bc856d Compare June 24, 2021 09:28
@tamasvajk tamasvajk changed the title C#: Convert System.Console.Read* local flow source to CSV C#: Start using CSV based flow models Jun 24, 2021
m = responseClass.getABinaryWriteMethod()
|
// Calls to these methods, or overrides of them
this.getExpr() = m.getAnOverrider*().getParameter(0).getAnAssignedArgument()
Copy link
Contributor Author

@tamasvajk tamasvajk Jun 24, 2021

Choose a reason for hiding this comment

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

We can't express getAnOverrider* in CSV, but anyways, HttpResponse is sealed.

@tamasvajk tamasvajk force-pushed the feature/try-csv-source-models branch from d7cc6e9 to e05ef6c Compare June 24, 2021 10:00
@github github deleted a comment from github-actions bot Jun 24, 2021
@tamasvajk tamasvajk force-pushed the feature/try-csv-source-models branch 2 times, most recently from e123bcc to 521d018 Compare June 25, 2021 08:00
@tamasvajk tamasvajk force-pushed the feature/try-csv-source-models branch from a17f3fc to 6fceb1b Compare June 25, 2021 11:14
@tamasvajk tamasvajk force-pushed the feature/try-csv-source-models branch from 3ba39f1 to 23badc4 Compare June 28, 2021 09:21
@tamasvajk
Copy link
Contributor Author

The PR was rebased to fix merge conflicts.

@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. The following differences were found:

java

Generated file changes for java

  • Changes to framework-coverage-java.rst:
-    Framework / library,Package,Remote flow sources,Taint & value steps,Sinks (total),`CWE‑022` :sub:`Path injection`,`CWE‑036` :sub:`Path traversal`,`CWE‑079` :sub:`Cross-site scripting`,`CWE‑089` :sub:`SQL injection`,`CWE‑090` :sub:`LDAP injection`,`CWE‑094` :sub:`Code injection`,`CWE‑319` :sub:`Cleartext transmission`
+    Framework / library,Package,Flow sources,Taint & value steps,Sinks (total),`CWE‑022` :sub:`Path injection`,`CWE‑036` :sub:`Path traversal`,`CWE‑079` :sub:`Cross-site scripting`,`CWE‑089` :sub:`SQL injection`,`CWE‑090` :sub:`LDAP injection`,`CWE‑094` :sub:`Code injection`,`CWE‑319` :sub:`Cleartext transmission`

@tamasvajk tamasvajk force-pushed the feature/try-csv-source-models branch from 8954c5d to 3b58569 Compare June 28, 2021 09:29
@tamasvajk
Copy link
Contributor Author

The comment #6148 (comment) is specific to this PR. main and this branch produce different CSV files, so the changes are shown on the PR.

The "remote vs all flows" change will generate some noise on other PRs too: PRs that were opened before this change will report different coverage files in the previous run of the job than in the current one, so a comment will be added to them. (Usually the comment that a recent change removed a previously reported coverage change will be added.)

@tamasvajk tamasvajk marked this pull request as ready for review June 28, 2021 13:29
@tamasvajk tamasvajk requested a review from a team as a code owner June 28, 2021 13:29
@tamasvajk tamasvajk added the no-change-note-required This PR does not need a change note label Jun 28, 2021
@yo-h yo-h requested a review from hvitved June 28, 2021 19:41
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. Thanks for starting this work Tamas.

@tamasvajk tamasvajk dismissed a stale review via a469cf0 June 30, 2021 07:26
hvitved
hvitved previously approved these changes Jun 30, 2021
@tamasvajk tamasvajk force-pushed the feature/try-csv-source-models branch from 6754980 to 0946ae2 Compare June 30, 2021 09:39
@tamasvajk tamasvajk merged commit 10a6089 into github:main Jun 30, 2021
owen-mc added a commit to owen-mc/codeql that referenced this pull request Oct 11, 2023
owen-mc added a commit to owen-mc/codeql that referenced this pull request Oct 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C# no-change-note-required This PR does not need a change note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants