Merge branch 'main' into experimental-strong-params

Author: thiggy1342

cpp/ql/lib/change-notes/2022-06-24-unique-variable.md

@@ -0,0 +1,4 @@
+---
+category: fix
+---
+* Under certain circumstances a variable declaration that is not also a definition could be associated with a `Variable` that did not have the definition as a `VariableDeclarationEntry`. This is now fixed, and a unique `Variable` will exist that has both the declaration and the definition as a `VariableDeclarationEntry`.

cpp/ql/lib/semmle/code/cpp/Element.qll

@@ -6,6 +6,7 @@
 import semmle.code.cpp.Location
 private import semmle.code.cpp.Enclosing
 private import semmle.code.cpp.internal.ResolveClass
+private import semmle.code.cpp.internal.ResolveGlobalVariable
 
 /**
  * Get the `Element` that represents this `@element`.
@@ -28,9 +29,12 @@ Element mkElement(@element e) { unresolveElement(result) = e }
 pragma[inline]
 @element unresolveElement(Element e) {
   not result instanceof @usertype and
+  not result instanceof @variable and
   result = e
   or
   e = resolveClass(result)
+  or
+  e = resolveGlobalVariable(result)
 }
 
 /**

cpp/ql/lib/semmle/code/cpp/Variable.qll

@@ -6,6 +6,7 @@ import semmle.code.cpp.Element
 import semmle.code.cpp.exprs.Access
 import semmle.code.cpp.Initializer
 private import semmle.code.cpp.internal.ResolveClass
+private import semmle.code.cpp.internal.ResolveGlobalVariable
 
 /**
  * A C/C++ variable. For example, in the following code there are four
@@ -32,6 +33,8 @@ private import semmle.code.cpp.internal.ResolveClass
  * can have multiple declarations.
  */
 class Variable extends Declaration, @variable {
+  Variable() { isVariable(underlyingElement(this)) }
+
   override string getAPrimaryQlClass() { result = "Variable" }
 
   /** Gets the initializer of this variable, if any. */

cpp/ql/lib/semmle/code/cpp/internal/ResolveGlobalVariable.qll

@@ -0,0 +1,60 @@
+private predicate hasDefinition(@globalvariable g) {
+  exists(@var_decl vd | var_decls(vd, g, _, _, _) | var_def(vd))
+}
+
+pragma[noinline]
+private predicate onlyOneCompleteGlobalVariableExistsWithMangledName(@mangledname name) {
+  strictcount(@globalvariable g | hasDefinition(g) and mangled_name(g, name)) = 1
+}
+
+/** Holds if `g` is a unique global variable with a definition named `name`. */
+pragma[noinline]
+private predicate isGlobalWithMangledNameAndWithDefinition(@mangledname name, @globalvariable g) {
+  hasDefinition(g) and
+  mangled_name(g, name) and
+  onlyOneCompleteGlobalVariableExistsWithMangledName(name)
+}
+
+/** Holds if `g` is a global variable without a definition named `name`. */
+pragma[noinline]
+private predicate isGlobalWithMangledNameAndWithoutDefinition(@mangledname name, @globalvariable g) {
+  not hasDefinition(g) and
+  mangled_name(g, name)
+}
+
+/**
+ * Holds if `incomplete` is a global variable without a definition, and there exists
+ * a unique global variable `complete` with the same name that does have a definition.
+ */
+private predicate hasTwinWithDefinition(@globalvariable incomplete, @globalvariable complete) {
+  exists(@mangledname name |
+    not variable_instantiation(incomplete, complete) and
+    isGlobalWithMangledNameAndWithoutDefinition(name, incomplete) and
+    isGlobalWithMangledNameAndWithDefinition(name, complete)
+  )
+}
+
+import Cached
+
+cached
+private module Cached {
+  /**
+   * If `v` is a global variable without a definition, and there exists a unique
+   * global variable with the same name that does have a definition, then the
+   * result is that unique global variable. Otherwise, the result is `v`.
+   */
+  cached
+  @variable resolveGlobalVariable(@variable v) {
+    hasTwinWithDefinition(v, result)
+    or
+    not hasTwinWithDefinition(v, _) and
+    result = v
+  }
+
+  cached
+  predicate isVariable(@variable v) {
+    not v instanceof @globalvariable
+    or
+    v = resolveGlobalVariable(_)
+  }
+}

cpp/ql/src/Likely Bugs/Memory Management/ReturnStackAllocatedMemory.ql

@@ -74,13 +74,12 @@ class ReturnStackAllocatedMemoryConfig extends MustFlowConfiguration {
 
 from
   MustFlowPathNode source, MustFlowPathNode sink, VariableAddressInstruction var,
-  ReturnStackAllocatedMemoryConfig conf, Function f
+  ReturnStackAllocatedMemoryConfig conf
 where
-  conf.hasFlowPath(source, sink) and
+  conf.hasFlowPath(pragma[only_bind_into](source), pragma[only_bind_into](sink)) and
   source.getNode().asInstruction() = var and
   // Only raise an alert if we're returning from the _same_ callable as the on that
   // declared the stack variable.
-  var.getEnclosingFunction() = pragma[only_bind_into](f) and
-  sink.getNode().getEnclosingCallable() = pragma[only_bind_into](f)
+  var.getEnclosingFunction() = sink.getNode().getEnclosingCallable()
 select sink.getNode(), source, sink, "May return stack-allocated memory from $@.", var.getAst(),
   var.getAst().toString()

cpp/ql/src/Security/CWE/CWE-078/ExecTainted.ql

@@ -77,7 +77,7 @@ class ExecState extends DataFlow::FlowState {
   ExecState() {
     this =
       "ExecState (" + fst.getLocation() + " | " + fst + ", " + snd.getLocation() + " | " + snd + ")" and
-    interestingConcatenation(fst, snd)
+    interestingConcatenation(pragma[only_bind_into](fst), pragma[only_bind_into](snd))
   }
 
   DataFlow::Node getFstNode() { result = fst }

cpp/ql/test/library-tests/variables/global/variables.expected

@@ -4,11 +4,7 @@
 | c.c:6:5:6:6 | ls | array of 4 {int} | 1 |
 | c.c:8:5:8:7 | iss | array of 4 {array of 2 {int}} | 1 |
 | c.c:12:11:12:11 | i | typedef {int} as "int_alias" | 1 |
-| c.h:4:12:4:13 | ks | array of {int} | 1 |
-| c.h:8:12:8:14 | iss | array of {array of 2 {int}} | 1 |
-| c.h:10:12:10:12 | i | int | 1 |
 | d.cpp:3:7:3:8 | xs | array of {int} | 1 |
-| d.h:3:14:3:15 | xs | array of 2 {int} | 1 |
 | file://:0:0:0:0 | (unnamed parameter 0) | reference to {const {struct __va_list_tag}} | 1 |
 | file://:0:0:0:0 | (unnamed parameter 0) | rvalue reference to {struct __va_list_tag} | 1 |
 | file://:0:0:0:0 | fp_offset | unsigned int | 1 |

csharp/tools/tracing-config.lua

@@ -2,29 +2,75 @@ function RegisterExtractorPack(id)
     local extractor = GetPlatformToolsDirectory() ..
                           'Semmle.Extraction.CSharp.Driver'
     if OperatingSystem == 'windows' then extractor = extractor .. '.exe' end
+
+    function DotnetMatcherBuild(compilerName, compilerPath, compilerArguments,
+                                _languageId)
+        if compilerName ~= 'dotnet' and compilerName ~= 'dotnet.exe' then
+            return nil
+        end
+
+        -- The dotnet CLI has the following usage instructions:
+        -- dotnet [sdk-options] [command] [command-options] [arguments]
+        -- we are interested in dotnet build, which has the following usage instructions:
+        -- dotnet [options] build [<PROJECT | SOLUTION>...]
+        -- For now, parse the command line as follows:
+        -- Everything that starts with `-` (or `/`) will be ignored.
+        -- The first non-option argument is treated as the command.
+        -- if that's `build`, we append `/p:UseSharedCompilation=false` to the command line,
+        -- otherwise we do nothing.
+        local match = false
+        local argv = compilerArguments.argv
+        if OperatingSystem == 'windows' then
+            -- let's hope that this split matches the escaping rules `dotnet` applies to command line arguments
+            -- or, at least, that it is close enough
+            argv =
+                NativeArgumentsToArgv(compilerArguments.nativeArgumentPointer)
+        end
+        for i, arg in ipairs(argv) do
+            -- dotnet options start with either - or / (both are legal)
+            local firstCharacter = string.sub(arg, 1, 1)
+            if not (firstCharacter == '-') and not (firstCharacter == '/') then
+                Log(1, 'Dotnet subcommand detected: %s', arg)
+                if arg == 'build' then match = true end
+                break
+            end
+        end
+        if match then
+            return {
+                order = ORDER_REPLACE,
+                invocation = BuildExtractorInvocation(id, compilerPath,
+                                                      compilerPath,
+                                                      compilerArguments, nil, {
+                    '/p:UseSharedCompilation=false'
+                })
+            }
+        end
+        return nil
+    end
+
     local windowsMatchers = {
+        DotnetMatcherBuild,
         CreatePatternMatcher({'^dotnet%.exe$'}, MatchCompilerName, extractor, {
             prepend = {'--dotnetexec', '--cil'},
             order = ORDER_BEFORE
         }),
         CreatePatternMatcher({'^csc.*%.exe$'}, MatchCompilerName, extractor, {
             prepend = {'--compiler', '"${compiler}"', '--cil'},
             order = ORDER_BEFORE
-
         }),
         CreatePatternMatcher({'^fakes.*%.exe$', 'moles.*%.exe'},
                              MatchCompilerName, nil, {trace = false})
     }
     local posixMatchers = {
-        CreatePatternMatcher({'^mcs%.exe$', '^csc%.exe$'}, MatchCompilerName,
+        DotnetMatcherBuild,
+        CreatePatternMatcher({'^mono', '^dotnet$'}, MatchCompilerName,
                              extractor, {
-            prepend = {'--compiler', '"${compiler}"', '--cil'},
+            prepend = {'--dotnetexec', '--cil'},
             order = ORDER_BEFORE
-
         }),
-        CreatePatternMatcher({'^mono', '^dotnet$'}, MatchCompilerName,
+        CreatePatternMatcher({'^mcs%.exe$', '^csc%.exe$'}, MatchCompilerName,
                              extractor, {
-            prepend = {'--dotnetexec', '--cil'},
+            prepend = {'--compiler', '"${compiler}"', '--cil'},
             order = ORDER_BEFORE
         }), function(compilerName, compilerPath, compilerArguments, _languageId)
             if MatchCompilerName('^msbuild$', compilerName, compilerPath,
@@ -49,7 +95,6 @@ function RegisterExtractorPack(id)
     else
         return posixMatchers
     end
-
 end
 
 -- Return a list of minimum supported versions of the configuration file format