Merge branch 'main' into select-the-right-node-for-flow-sources

MathiasVP · MathiasVP · commit d54ab640c770 · 2023-10-11T10:17:10.000+01:00
diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/AssemblyCache.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/AssemblyCache.cs
@@ -70,7 +70,8 @@ private void IndexReferences()
             foreach (var info in assemblyInfoByFileName.Values
                 .OrderBy(info => info.Name)
                 .ThenBy(info => info.NetCoreVersion ?? emptyVersion)
-                .ThenBy(info => info.Version ?? emptyVersion))
+                .ThenBy(info => info.Version ?? emptyVersion)
+                .ThenBy(info => info.Filename))
             {
                 foreach (var index in info.IndexStrings)
                 {
diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyManager.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyManager.cs
@@ -322,7 +322,11 @@ private void ResolveConflicts()
             }
 
             var emptyVersion = new Version(0, 0);
-            sortedReferences = sortedReferences.OrderBy(r => r.NetCoreVersion ?? emptyVersion).ThenBy(r => r.Version ?? emptyVersion).ToList();
+            sortedReferences = sortedReferences
+                .OrderBy(r => r.NetCoreVersion ?? emptyVersion)
+                .ThenBy(r => r.Version ?? emptyVersion)
+                .ThenBy(r => r.Filename)
+                .ToList();
 
             var finalAssemblyList = new Dictionary<string, AssemblyInfo>();
 
diff --git a/csharp/ql/integration-tests/all-platforms/autobuild/Files.expected b/csharp/ql/integration-tests/all-platforms/autobuild/Files.expected
@@ -0,0 +1,3 @@
+| Program.cs:0:0:0:0 | Program.cs |
+| obj/Debug/net5.0/.NETCoreApp,Version=v5.0.AssemblyAttributes.cs:0:0:0:0 | obj/Debug/net5.0/.NETCoreApp,Version=v5.0.AssemblyAttributes.cs |
+| obj/Debug/net5.0/autobuild.AssemblyInfo.cs:0:0:0:0 | obj/Debug/net5.0/autobuild.AssemblyInfo.cs |
diff --git a/csharp/ql/integration-tests/all-platforms/autobuild/Files.ql b/csharp/ql/integration-tests/all-platforms/autobuild/Files.ql
@@ -0,0 +1,5 @@
+import csharp
+
+from File f
+where f.fromSource()
+select f
diff --git a/csharp/ql/integration-tests/all-platforms/autobuild/Program.cs b/csharp/ql/integration-tests/all-platforms/autobuild/Program.cs
@@ -0,0 +1 @@
+﻿var dummy = "dummy";
diff --git a/csharp/ql/integration-tests/all-platforms/autobuild/autobuild.csproj b/csharp/ql/integration-tests/all-platforms/autobuild/autobuild.csproj
@@ -0,0 +1,14 @@
+<Project Sdk="Microsoft.NET.Sdk">
+
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <TargetFramework>net5.0</TargetFramework>
+    <ImplicitUsings>enable</ImplicitUsings>
+    <Nullable>enable</Nullable>
+  </PropertyGroup>
+
+ <Target Name="DeleteBinObjFolders" BeforeTargets="Clean">
+   <RemoveDir Directories=".\bin" />
+   <RemoveDir Directories=".\obj" />
+ </Target>
+</Project>
diff --git a/csharp/ql/integration-tests/all-platforms/autobuild/global.json b/csharp/ql/integration-tests/all-platforms/autobuild/global.json
@@ -0,0 +1,5 @@
+{
+    "sdk": {
+        "version": "5.0.408"
+    }
+}
diff --git a/csharp/ql/integration-tests/all-platforms/autobuild/test.py b/csharp/ql/integration-tests/all-platforms/autobuild/test.py
@@ -0,0 +1,3 @@
+from create_database_utils import *
+
+run_codeql_database_create([], lang="csharp", extra_args=["--extractor-option=cil=false"])
diff --git a/javascript/ql/src/Security/CWE-400/RemotePropertyInjection.expected b/javascript/ql/src/Security/CWE-400/RemotePropertyInjection.expected
diff --git a/python/ql/lib/change-notes/2023-10-10-api-graphs-import-star.md b/python/ql/lib/change-notes/2023-10-10-api-graphs-import-star.md
@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Added better support for API graphs when encountering `from ... import *`. For example in the code `from foo import *; Bar()`, we will now find a result for `API::moduleImport("foo").getMember("Bar").getACall()`
diff --git a/python/ql/lib/semmle/python/dataflow/new/internal/LocalSources.qll b/python/ql/lib/semmle/python/dataflow/new/internal/LocalSources.qll
@@ -11,6 +11,7 @@ import DataFlowPublic
 private import DataFlowPrivate
 private import semmle.python.internal.CachedStages
 private import semmle.python.internal.Awaited
+private import semmle.python.dataflow.new.internal.ImportStar
 
 /**
  * A data flow node that is a source of local flow. This includes things like
@@ -39,6 +40,22 @@ class LocalSourceNode extends Node {
     this instanceof ExprNode and
     not simpleLocalFlowStepForTypetracking(_, this)
     or
+    // For `from foo import *; foo_function()`, we want to let the variables we think
+    // could originate in `foo` (such as `foo_function`) to be available in the API
+    // graph. This requires them to be local sources. They would not be from the code
+    // just above, since the CFG node has flow going into it from its corresponding
+    // `GlobalSsaVariable`. (a different work-around is to change API graphs to not rely
+    // as heavily on LocalSourceNode; I initially tried this, but it relied on a lot of
+    // copy-pasted code, and it requires some non-trivial deprecation for downgrading
+    // the result type of `.asSource()` to DataFlow::Node, so we've opted for this
+    // approach instead).
+    //
+    // Note: This is only needed at the module level -- uses inside functions appear as
+    // LocalSourceNodes as we expect.
+    //
+    // TODO: When rewriting SSA, we should be able to remove this workaround
+    ImportStar::namePossiblyDefinedInImportStar(this.(ExprNode).getNode(), _, any(Module m))
+    or
     // We include all module variable nodes, as these act as stepping stones between writes and
     // reads of global variables. Without them, type tracking based on `LocalSourceNode`s would be
     // unable to track across global variables.
diff --git a/python/ql/test/library-tests/ApiGraphs/py3/test_import_star.py b/python/ql/test/library-tests/ApiGraphs/py3/test_import_star.py
@@ -2,9 +2,11 @@
 
 from unknown import * #$ use=moduleImport("unknown")
 
-# Currently missing, as we do not consider `hello` to be a `LocalSourceNode`, since it has flow
-# going into it from its corresponding `GlobalSsaVariable`.
-hello() #$ MISSING: use=moduleImport("unknown").getMember("hello").getReturn()
+# This used to be missing, as we did not consider `hello` to be a `LocalSourceNode`,
+# since it has flow going into it from its corresponding `GlobalSsaVariable`.
+hello() #$ use=moduleImport("unknown").getMember("hello").getReturn()
+
+print(const_from_unknown) #$ use=moduleImport("unknown").getMember("const_from_unknown")
 
 # We don't want our analysis to think that either `non_module_member` or `outer_bar` can
 # come from `from unknown import *`

Original file line number	Diff line number	Diff line change
`@@ -70,7 +70,8 @@ private void IndexReferences()`
`70`	`70`	`foreach (var info in assemblyInfoByFileName.Values`
`71`	`71`	`.OrderBy(info => info.Name)`
`72`	`72`	`.ThenBy(info => info.NetCoreVersion ?? emptyVersion)`
`73`		`- .ThenBy(info => info.Version ?? emptyVersion))`
	`73`	`+ .ThenBy(info => info.Version ?? emptyVersion)`
	`74`	`+ .ThenBy(info => info.Filename))`
`74`	`75`	`{`
`75`	`76`	`foreach (var index in info.IndexStrings)`
`76`	`77`	`{`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+\| Program.cs:0:0:0:0 \| Program.cs \|`
	`2`	`+\| obj/Debug/net5.0/.NETCoreApp,Version=v5.0.AssemblyAttributes.cs:0:0:0:0 \| obj/Debug/net5.0/.NETCoreApp,Version=v5.0.AssemblyAttributes.cs \|`
	`3`	`+\| obj/Debug/net5.0/autobuild.AssemblyInfo.cs:0:0:0:0 \| obj/Debug/net5.0/autobuild.AssemblyInfo.cs \|`
-Original file line number
+Diff line change
@@ @@ -0,0 +1,5 @@ @@
 +{
 +    "sdk": {
 +        "version": "5.0.408"
 +    }
 +}
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+from create_database_utils import *`
	`2`	`+`
	`3`	`+run_codeql_database_create([], lang="csharp", extra_args=["--extractor-option=cil=false"])`