C#: Introduce flow summaries for StringValues. #7465

michaelnebel · 2021-12-21T15:16:29Z

The purpose of this PR is to introduce flow summarries for the StringValues struct.
Currently, the FlowSummaries test cases do not include a stub for the StringValues struct.

That is, in this PR we

Generate a stub for Microsoft.Extensions.Primitives as this contains the StringValues struct.
Include the Microsoft.Extensions.Primitives project into the compilation used for FlowSummaries(Filtered)?.expected.
Generate Flow Summaries for StringValues.
Convert the flow summaries to CSV format and hardcode in Primitives.qll.
Delete flow implementation from LibraryTypeDataFlow.
Fix other testcases that include a stub for StringValues to use the generic stub (this includes copying the query files to the test directories as they need to "import csharp", where csharp is the test module that will ensure that flow for operatores defined in resources/stubs will be taken into account).

@hvitved : Added some description of the purpose of this PR.

github-actions · 2021-12-22T08:19:49Z

⚠️ 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:

-    Others,"``Dapper``, ``Microsoft.ApplicationBlocks.Data``, ``Microsoft.VisualBasic``, ``MySql.Data.MySqlClient``, ``Newtonsoft.Json``",,95,131,
+    Others,"``Dapper``, ``Microsoft.ApplicationBlocks.Data``, ``Microsoft.Extensions.Primitives``, ``Microsoft.VisualBasic``, ``MySql.Data.MySqlClient``, ``Newtonsoft.Json``",,149,131,
-    Totals,,3,2438,353,5
+    Totals,,3,2492,353,5

Changes to framework-coverage-csharp.csv:

+ Microsoft.Extensions.Primitives,,,54,,,,,,,54,

github-actions · 2021-12-22T12:44:39Z

⚠️ 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:

-    Others,"``Dapper``, ``Microsoft.ApplicationBlocks.Data``, ``Microsoft.VisualBasic``, ``MySql.Data.MySqlClient``, ``Newtonsoft.Json``",,95,131,
+    Others,"``Dapper``, ``Microsoft.ApplicationBlocks.Data``, ``Microsoft.Extensions.Primitives``, ``Microsoft.VisualBasic``, ``MySql.Data.MySqlClient``, ``Newtonsoft.Json``",,149,131,
-    Totals,,3,2438,353,5
+    Totals,,3,2492,353,5

Changes to framework-coverage-csharp.csv:

+ Microsoft.Extensions.Primitives,,,54,,,,,,,54,

michaelnebel · 2021-12-22T12:45:38Z

csharp/ql/test/query-tests/Security Features/CWE-601/UrlRedirect/UrlRedirect.expected

@@ -4,15 +4,12 @@ edges
 | UrlRedirect.cs:37:44:37:66 | access to property QueryString : NameValueCollection | UrlRedirect.cs:37:44:37:74 | access to indexer |
 | UrlRedirect.cs:38:47:38:69 | access to property QueryString : NameValueCollection | UrlRedirect.cs:38:47:38:77 | access to indexer |
 | UrlRedirectCore.cs:13:44:13:48 | value : String | UrlRedirectCore.cs:16:22:16:26 | access to parameter value |
-| UrlRedirectCore.cs:13:44:13:48 | value : String | UrlRedirectCore.cs:19:44:19:48 | access to parameter value : String |
-| UrlRedirectCore.cs:13:44:13:48 | value : String | UrlRedirectCore.cs:25:46:25:50 | access to parameter value : String |
+| UrlRedirectCore.cs:13:44:13:48 | value : String | UrlRedirectCore.cs:19:44:19:48 | call to operator implicit conversion |


@hvitved : This looks like some kind of edge contraction (where the access and the implicit operator call now have been turned into a single edge), but that the equivalent flows have been identified. Do you know more about this?

hvitved · 2021-12-22T12:54:17Z

csharp/ql/lib/semmle/code/csharp/frameworks/microsoft/extensions/Primitives.qll

+private import semmle.code.csharp.dataflow.ExternalFlow
+
+/** Data flow for `Microsoft.Extensions.Primitives.StringValues`. */
+private class MicrosoftVisualBasicCollectionFlowModelCsv extends SummaryModelCsv {


Copy-paste typo

Thank you! Will fix.

hvitved · 2021-12-22T12:54:17Z

csharp/ql/test/library-tests/dataflow/library/FlowSummaries.expected

@@ -1,3 +1,66 @@
+| Microsoft.Extensions.Primitives;StringTokenizer;false;GetEnumerator;();;Element of Argument[-1];Property[System.Collections.Generic.IEnumerator<>.Current] of ReturnValue;value |


Why were these not in the output before?

The struct StringTokenizer stub has just been included as a part of Microsoft.Extensions.Primitives (note that these are derived summaries).

Why has FlowSummariesFiltered.expected then changed?

Microsoft.Extensions.Primitives was included because it contains the StringValues struct. Before the FlowSummaries test didn't know about this struct and couldn't generate any test output for it.

github-actions · 2021-12-22T14:03:34Z

⚠️ 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:

-    Others,"``Dapper``, ``Microsoft.ApplicationBlocks.Data``, ``Microsoft.VisualBasic``, ``MySql.Data.MySqlClient``, ``Newtonsoft.Json``",,95,131,
+    Others,"``Dapper``, ``Microsoft.ApplicationBlocks.Data``, ``Microsoft.Extensions.Primitives``, ``Microsoft.VisualBasic``, ``MySql.Data.MySqlClient``, ``Newtonsoft.Json``",,149,131,
-    Totals,,3,2438,353,5
+    Totals,,3,2492,353,5

Changes to framework-coverage-csharp.csv:

+ Microsoft.Extensions.Primitives,,,54,,,,,,,54,

github-actions · 2021-12-22T14:30:27Z

⚠️ 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:

-    Others,"``Dapper``, ``Microsoft.ApplicationBlocks.Data``, ``Microsoft.VisualBasic``, ``MySql.Data.MySqlClient``, ``Newtonsoft.Json``",,95,131,
+    Others,"``Dapper``, ``Microsoft.ApplicationBlocks.Data``, ``Microsoft.Extensions.Primitives``, ``Microsoft.VisualBasic``, ``MySql.Data.MySqlClient``, ``Newtonsoft.Json``",,149,131,
-    Totals,,3,2438,353,5
+    Totals,,3,2492,353,5

Changes to framework-coverage-csharp.csv:

+ Microsoft.Extensions.Primitives,,,54,,,,,,,54,

…tives.StringValues.

…mport csharp (test module).

…t.Extensions.Primitives.

…rt csharp test module.

github-actions · 2021-12-22T14:58:12Z

⚠️ 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:

-    Others,"``Dapper``, ``Microsoft.ApplicationBlocks.Data``, ``Microsoft.VisualBasic``, ``MySql.Data.MySqlClient``, ``Newtonsoft.Json``",,95,131,
+    Others,"``Dapper``, ``Microsoft.ApplicationBlocks.Data``, ``Microsoft.Extensions.Primitives``, ``Microsoft.VisualBasic``, ``MySql.Data.MySqlClient``, ``Newtonsoft.Json``",,149,131,
-    Totals,,3,2438,353,5
+    Totals,,3,2492,353,5

Changes to framework-coverage-csharp.csv:

+ Microsoft.Extensions.Primitives,,,54,,,,,,,54,

github-actions bot added the C# label Dec 21, 2021

michaelnebel force-pushed the csharp-stringvalues-csv branch from 9898525 to cfffe9e Dec 22, 2021

michaelnebel reviewed Dec 22, 2021

View changes

hvitved reviewed Dec 22, 2021

View changes

michaelnebel force-pushed the csharp-stringvalues-csv branch from 824f147 to 20dbeb1 Dec 22, 2021

michaelnebel force-pushed the csharp-stringvalues-csv branch from 20dbeb1 to e7ba73e Dec 22, 2021

michaelnebel added 10 commits Dec 22, 2021

C#: Add stubs for Microsoft.Extensions.Primitives.

3aa9aa2

C#: Reduce the amount of trash flow summaries produced for StringValues.

5bf2eb0

C#: Update flow summaries for StringValues.

e449416

C#: Convert and cleanup flow summaries for Microsoft.Extensions.Primi…

d2d7b43

…tives.StringValues.

C#: Replace query reference for UrlDirect with a copy as we need to i…

6c9c932

…mport csharp (test module).

C#: Replace StringValues stub from stubs.cs with the stub in Microsof…

ed2c3bf

…t.Extensions.Primitives.

C#: Update the UrlRedirect expected file.

745527b

C#: Take a copy of XSS.ql instead of referring it, as we need to impo…

2824066

…rt csharp test module.

C#: Only use stubs for XSS test.

8b50118

C#: Update the XSS expected file.

7d9b85b

michaelnebel force-pushed the csharp-stringvalues-csv branch from e7ba73e to 7d9b85b Dec 22, 2021

github / codeql Public

C#: Introduce flow summaries for StringValues. #7465

C#: Introduce flow summaries for StringValues. #7465

michaelnebel commented Dec 21, 2021 •

edited

github-actions bot commented Dec 22, 2021

csharp

Generated file changes for csharp

github-actions bot commented Dec 22, 2021

csharp

Generated file changes for csharp

michaelnebel Dec 22, 2021

hvitved Dec 22, 2021

michaelnebel Dec 22, 2021

hvitved Dec 22, 2021

michaelnebel Dec 22, 2021

hvitved Dec 22, 2021

michaelnebel Dec 22, 2021

github-actions bot commented Dec 22, 2021

csharp

Generated file changes for csharp

github-actions bot commented Dec 22, 2021

csharp

Generated file changes for csharp

github-actions bot commented Dec 22, 2021

csharp

Generated file changes for csharp

		@@ -1,3 +1,66 @@
		\| Microsoft.Extensions.Primitives;StringTokenizer;false;GetEnumerator;();;Element of Argument[-1];Property[System.Collections.Generic.IEnumerator<>.Current] of ReturnValue;value \|

github / codeql Public

C#: Introduce flow summaries for StringValues. #7465

Are you sure you want to change the base?

C#: Introduce flow summaries for StringValues. #7465

Conversation

michaelnebel commented Dec 21, 2021 • edited

github-actions bot commented Dec 22, 2021

csharp

Generated file changes for csharp

github-actions bot commented Dec 22, 2021

csharp

Generated file changes for csharp

michaelnebel Dec 22, 2021

Choose a reason for hiding this comment

hvitved Dec 22, 2021

Choose a reason for hiding this comment

michaelnebel Dec 22, 2021

Choose a reason for hiding this comment

hvitved Dec 22, 2021

Choose a reason for hiding this comment

michaelnebel Dec 22, 2021

Choose a reason for hiding this comment

hvitved Dec 22, 2021

Choose a reason for hiding this comment

michaelnebel Dec 22, 2021

Choose a reason for hiding this comment

github-actions bot commented Dec 22, 2021

csharp

Generated file changes for csharp

github-actions bot commented Dec 22, 2021

csharp

Generated file changes for csharp

github-actions bot commented Dec 22, 2021

csharp

Generated file changes for csharp

michaelnebel commented Dec 21, 2021 •

edited