Merge branch 'main' into aegilops/js/insecure-helmet-middleware

.github/workflows/compile-queries.yml

@@ -29,6 +29,8 @@ jobs:
           key: all-queries
       - name: check formatting
         run: find shared */ql -type f \( -name "*.qll" -o -name "*.ql" \) -print0 | xargs -0 -n 3000 -P 10 codeql query format -q --check-only
+      - name: Omit DatabaseQualityDiagnostics.ql from compile checking # Remove me once CodeQL 2.18.0 is released!
+        run: mv java/ql/src/Telemetry/DatabaseQualityDiagnostics.ql{,.hidden}
       - name: compile queries - check-only
         # run with --check-only if running in a PR (github.sha != main)
         if : ${{ github.event_name == 'pull_request' }}
@@ -39,3 +41,6 @@ jobs:
         if : ${{ github.event_name != 'pull_request' }}
         shell: bash
         run: codeql query compile -q -j0 */ql/{src,examples} --keep-going --warnings=error --compilation-cache "${{ steps.query-cache.outputs.cache-dir }}" --compilation-cache-size=500
+      - name: Restore DatabaseQualityDiagnostics.ql after compile checking # Remove me once CodeQL 2.18.0 is released
+        run: mv java/ql/src/Telemetry/DatabaseQualityDiagnostics.ql{.hidden,}
+

cpp/ql/lib/CHANGELOG.md

@@ -1,3 +1,15 @@
+## 1.2.0
+
+### New Features
+
+* The syntax for models-as-data rows has been extended to make it easier to select sources, sinks, and summaries that involve templated functions and classes. Additionally, the syntax has also been extended to make it easier to specify models with arbitrary levels of indirection. See `dataflow/ExternalFlow.qll` for the updated documentation and specification for the model format.
+* It is now possible to extend the classes `AllocationFunction` and `DeallocationFunction` via data extensions. Extensions of these classes should be added to the `lib/ext/allocation` and `lib/ext/deallocation` directories respectively.
+
+### Minor Analysis Improvements
+
+* The queries "Potential double free" (`cpp/double-free`) and "Potential use after free" (`cpp/use-after-free`) now produce fewer false positives.
+* The "Guards" library (`semmle.code.cpp.controlflow.Guards`) now also infers guards from calls to the builtin operation `__builtin_expect`. As a result, some queries may produce fewer false positives.
+
 ## 1.1.1
 
 No user-facing changes.

cpp/ql/lib/change-notes/released/1.2.0.md

@@ -0,0 +1,11 @@
+## 1.2.0
+
+### New Features
+
+* The syntax for models-as-data rows has been extended to make it easier to select sources, sinks, and summaries that involve templated functions and classes. Additionally, the syntax has also been extended to make it easier to specify models with arbitrary levels of indirection. See `dataflow/ExternalFlow.qll` for the updated documentation and specification for the model format.
+* It is now possible to extend the classes `AllocationFunction` and `DeallocationFunction` via data extensions. Extensions of these classes should be added to the `lib/ext/allocation` and `lib/ext/deallocation` directories respectively.
+
+### Minor Analysis Improvements
+
+* The queries "Potential double free" (`cpp/double-free`) and "Potential use after free" (`cpp/use-after-free`) now produce fewer false positives.
+* The "Guards" library (`semmle.code.cpp.controlflow.Guards`) now also infers guards from calls to the builtin operation `__builtin_expect`. As a result, some queries may produce fewer false positives.

cpp/ql/lib/codeql-pack.release.yml

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 1.1.1
+lastReleaseVersion: 1.2.0

cpp/ql/lib/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/cpp-all
-version: 1.1.2-dev
+version: 1.2.1-dev
 groups: cpp
 dbscheme: semmlecode.cpp.dbscheme
 extractor: cpp

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll

@@ -104,7 +104,7 @@ predicate hasRawIndirectInstruction(Instruction instr, int indirectionIndex) {
 
 cached
 private newtype TDefImpl =
-  TDefAddressImpl(BaseIRVariable v) or
+  TDefAddressImpl(BaseSourceVariable v) or
   TDirectDefImpl(Operand address, int indirectionIndex) {
     isDef(_, _, address, _, _, indirectionIndex)
   } or
@@ -325,9 +325,9 @@ private Instruction getInitializationTargetAddress(IRVariable v) {
   )
 }
 
-/** An initial definition of an `IRVariable`'s address. */
-private class DefAddressImpl extends DefImpl, TDefAddressImpl {
-  BaseIRVariable v;
+/** An initial definition of an SSA variable address. */
+abstract private class DefAddressImpl extends DefImpl, TDefAddressImpl {
+  BaseSourceVariable v;
 
   DefAddressImpl() {
     this = TDefAddressImpl(v) and
@@ -342,6 +342,19 @@ private class DefAddressImpl extends DefImpl, TDefAddressImpl {
 
   final override Node0Impl getValue() { none() }
 
+  override Cpp::Location getLocation() { result = v.getLocation() }
+
+  final override SourceVariable getSourceVariable() {
+    result.getBaseVariable() = v and
+    result.getIndirection() = 0
+  }
+
+  final override BaseSourceVariable getBaseSourceVariable() { result = v }
+}
+
+private class DefVariableAddressImpl extends DefAddressImpl {
+  override BaseIRVariable v;
+
   final override predicate hasIndexInBlock(IRBlock block, int index) {
     exists(IRVariable var | var = v.getIRVariable() |
       block.getInstruction(index) = getInitializationTargetAddress(var)
@@ -353,15 +366,14 @@ private class DefAddressImpl extends DefImpl, TDefAddressImpl {
       index = 0
     )
   }
+}
 
-  override Cpp::Location getLocation() { result = v.getIRVariable().getLocation() }
+private class DefCallAddressImpl extends DefAddressImpl {
+  override BaseCallVariable v;
 
-  final override SourceVariable getSourceVariable() {
-    result.getBaseVariable() = v and
-    result.getIndirection() = 0
+  final override predicate hasIndexInBlock(IRBlock block, int index) {
+    block.getInstruction(index) = v.getCallInstruction()
   }
-
-  final override BaseSourceVariable getBaseSourceVariable() { result = v }
 }
 
 private class DirectDef extends DefImpl, TDirectDefImpl {

cpp/ql/src/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 1.0.3
+
+No user-facing changes.
+
 ## 1.0.2
 
 No user-facing changes.

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

@@ -4,7 +4,7 @@
  * @kind problem
  * @problem.severity warning
  * @security-severity 9.3
- * @precision medium
+ * @precision high
  * @id cpp/unsafe-strncat
  * @tags reliability
  *       correctness

cpp/ql/src/change-notes/2024-07-08-unsafe-strncat-query.md

@@ -0,0 +1,4 @@
+---
+category: queryMetadata
+---
+* The precision of `cpp/unsafe-strncat` ("Potentially unsafe call to strncat") has been increased to `high`. As a result, it will be run by default as part of the Code Scanning suite.

cpp/ql/src/change-notes/released/1.0.3.md

@@ -0,0 +1,3 @@
+## 1.0.3
+
+No user-facing changes.

cpp/ql/src/codeql-pack.release.yml

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 1.0.2
+lastReleaseVersion: 1.0.3

cpp/ql/src/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/cpp-queries
-version: 1.0.3-dev
+version: 1.0.4-dev
 groups:
   - cpp
   - queries

cpp/ql/test/library-tests/dataflow/calls-as-ssa-variables/test.cpp

@@ -0,0 +1,29 @@
+namespace std
+{
+  struct ptrdiff_t;
+  struct input_iterator_tag
+  {
+  };
+  struct forward_iterator_tag : public input_iterator_tag
+  {
+  };
+}
+
+struct A
+{
+  using value_type = int;
+  using difference_type = std::ptrdiff_t;
+  using pointer = int*;
+  using reference = int&;
+  using iterator_category = std::forward_iterator_tag;
+};
+
+A get();
+
+void test()
+{
+  while (true)
+  {
+    auto &&x = get();
+  }
+}

cpp/ql/test/library-tests/dataflow/calls-as-ssa-variables/test.expected

@@ -0,0 +1,4 @@
+edges
+nodes
+subpaths
+#select

cpp/ql/test/library-tests/dataflow/calls-as-ssa-variables/test.ql

@@ -0,0 +1,23 @@
+/**
+ * @kind path-problem
+ */
+
+import semmle.code.cpp.ir.IR
+import semmle.code.cpp.dataflow.new.DataFlow
+import Flow::PathGraph
+
+module Config implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) {
+    source.asInstruction().(VariableAddressInstruction).getIRVariable() instanceof IRTempVariable
+  }
+
+  predicate isSink(DataFlow::Node sink) {
+    sink.asInstruction().(CallInstruction).getStaticCallTarget().hasName("get")
+  }
+}
+
+module Flow = DataFlow::Global<Config>;
+
+from Flow::PathNode source, Flow::PathNode sink
+where Flow::flowPath(source, sink)
+select sink.getNode(), source, sink, ""

cpp/ql/test/query-tests/Likely Bugs/Memory Management/SuspiciousCallToStrncat/SuspiciousCallToStrncat.expected

@@ -3,3 +3,5 @@
 | test.c:67:3:67:9 | call to strncat | Potentially unsafe call to strncat. |
 | test.c:75:3:75:9 | call to strncat | Potentially unsafe call to strncat. |
 | test.c:76:3:76:9 | call to strncat | Potentially unsafe call to strncat. |
+| test.c:91:3:91:9 | call to strncat | Potentially unsafe call to strncat. |
+| test.c:99:3:99:9 | call to strncat | Potentially unsafe call to strncat. |

cpp/ql/test/query-tests/Likely Bugs/Memory Management/SuspiciousCallToStrncat/test.c

@@ -82,3 +82,20 @@ void strncat_test5(char *s) {
   strncat(buf, s, len - strlen(buf) - 1); // GOOD
   strncat(buf, s, len - strlen(buf)); // GOOD
 }
+
+void strncat_test6() {
+  {
+  char dest[60];
+  dest[0] = '\0';
+  // Will write `dest[0 .. 5]`
+  strncat(dest, "small", sizeof(dest)); // GOOD [FALSE POSITIVE]
+  }
+
+  {
+  char dest[60];
+  memset(dest, 'a', sizeof(dest));
+  dest[54] = '\0';
+  // Will write `dest[54 .. 59]`
+  strncat(dest, "small", sizeof(dest)); // GOOD [FALSE POSITIVE]
+  }
+}

cpp/ql/test/query-tests/Security/CWE/CWE-416/semmle/tests/IteratorToExpiredContainer/IteratorToExpiredContainer.expected

@@ -3,4 +3,4 @@
 | test.cpp:702:27:702:27 | call to operator[] | This object is destroyed at the end of the full-expression. |
 | test.cpp:727:23:727:23 | call to operator[] | This object is destroyed at the end of the full-expression. |
 | test.cpp:735:23:735:23 | call to operator[] | This object is destroyed at the end of the full-expression. |
-| test.cpp:826:25:826:43 | pointer to ~HasBeginAndEnd output argument | This object is destroyed at the end of the full-expression. |
+| test.cpp:857:3:857:17 | pointer to ~PlusPlusReturnByValueIterator output argument | This object is destroyed at the end of the full-expression. |

cpp/ql/test/query-tests/Security/CWE/CWE-416/semmle/tests/IteratorToExpiredContainer/test.cpp

@@ -823,8 +823,38 @@ void test6()
 {
   while(getBool())
   {
-    for (const int& x : getHasBeginAndEnd()) // GOOD [FALSE POSITIVE]
+    for (const int& x : getHasBeginAndEnd()) // GOOD
     {
     }
   }
 }
+
+struct PlusPlusReturnByValueIterator
+{
+  using value_type = int;
+  using difference_type = std::ptrdiff_t;
+  using pointer = int *;
+  using reference = int &;
+  using iterator_category = std::forward_iterator_tag;
+
+  PlusPlusReturnByValueIterator();
+  PlusPlusReturnByValueIterator(PlusPlusReturnByValueIterator const &);
+
+  PlusPlusReturnByValueIterator operator++();
+  bool operator==(PlusPlusReturnByValueIterator other) const;
+  bool operator!=(PlusPlusReturnByValueIterator other) const;
+  reference operator*() const;
+  pointer operator->() const;
+
+  ~PlusPlusReturnByValueIterator();
+
+  PlusPlusReturnByValueIterator begin();
+};
+
+void test7()
+{
+  PlusPlusReturnByValueIterator it;
+  it.operator++(); // GOOD [FALSE POSITIVE]
+
+  it.begin();
+}

csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DotNet.cs

@@ -69,6 +69,11 @@ private string GetRestoreArgs(RestoreSettings restoreSettings)
                 args += " --force";
             }
 
+            if (restoreSettings.TargetWindows)
+            {
+                args += " /p:EnableWindowsTargeting=true";
+            }
+
             return args;
         }
 

csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FileProvider.cs

@@ -62,7 +62,7 @@ private string[] SelectTextFileNamesByExtension(string filetype, params string[]
         private string[] SelectTextFileNamesByName(string name)
         {
             var ret = allNonBinary.Value.SelectFileNamesByName(name).ToArray();
-            var ending = ret.Length == 0 ? "." : $": {string.Join(", ", ret.OrderBy(s => s))}.";
+            var ending = ret.Length == 0 ? "." : $": {string.Join(", ", ret)}.";
             logger.LogInfo($"Found {ret.Length} {name} files in {SourceDir}{ending}");
             return ret;
         }
@@ -91,7 +91,9 @@ private IEnumerable<FileInfo> SelectSmallFiles(IEnumerable<FileInfo> files)
         private FileInfo[] GetAllFiles()
         {
             logger.LogInfo($"Finding files in {SourceDir}...");
-            var files = SourceDir.GetFiles("*.*", new EnumerationOptions { RecurseSubdirectories = true });
+            var files = SourceDir
+                .GetFiles("*.*", new EnumerationOptions { RecurseSubdirectories = true })
+                .OrderBy(f => f.FullName);
 
             var filteredFiles = files.Where(f =>
             {

csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/IDotNet.cs

@@ -17,7 +17,7 @@ public interface IDotNet
         IList<string> GetNugetFeedsFromFolder(string folderPath);
     }
 
-    public record class RestoreSettings(string File, string PackageDirectory, bool ForceDotnetRefAssemblyFetching, string? PathToNugetConfig = null, bool ForceReevaluation = false);
+    public record class RestoreSettings(string File, string PackageDirectory, bool ForceDotnetRefAssemblyFetching, string? PathToNugetConfig = null, bool ForceReevaluation = false, bool TargetWindows = false);
 
     public partial record class RestoreResult(bool Success, IList<string> Output)
     {

csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs

@@ -225,10 +225,13 @@ private IEnumerable<string> RestoreSolutions(out DependencyContainer dependencie
             var successCount = 0;
             var nugetSourceFailures = 0;
             var assets = new Assets(logger);
+
+            var isWindows = fileContent.UseWindowsForms || fileContent.UseWpf;
+
             var projects = fileProvider.Solutions.SelectMany(solution =>
                 {
                     logger.LogInfo($"Restoring solution {solution}...");
-                    var res = dotnet.Restore(new(solution, PackageDirectory.DirInfo.FullName, ForceDotnetRefAssemblyFetching: true));
+                    var res = dotnet.Restore(new(solution, PackageDirectory.DirInfo.FullName, ForceDotnetRefAssemblyFetching: true, TargetWindows: isWindows));
                     if (res.Success)
                     {
                         successCount++;
@@ -258,6 +261,9 @@ private void RestoreProjects(IEnumerable<string> projects, out ConcurrentBag<Dep
             var successCount = 0;
             var nugetSourceFailures = 0;
             ConcurrentBag<DependencyContainer> collectedDependencies = [];
+
+            var isWindows = fileContent.UseWindowsForms || fileContent.UseWpf;
+
             var sync = new object();
             var projectGroups = projects.GroupBy(Path.GetDirectoryName);
             Parallel.ForEach(projectGroups, new ParallelOptions { MaxDegreeOfParallelism = DependencyManager.Threads }, projectGroup =>
@@ -266,7 +272,7 @@ private void RestoreProjects(IEnumerable<string> projects, out ConcurrentBag<Dep
                 foreach (var project in projectGroup)
                 {
                     logger.LogInfo($"Restoring project {project}...");
-                    var res = dotnet.Restore(new(project, PackageDirectory.DirInfo.FullName, ForceDotnetRefAssemblyFetching: true));
+                    var res = dotnet.Restore(new(project, PackageDirectory.DirInfo.FullName, ForceDotnetRefAssemblyFetching: true, TargetWindows: isWindows));
                     assets.AddDependenciesRange(res.AssetsFilePaths);
                     lock (sync)
                     {

csharp/ql/campaigns/Solorigate/lib/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 1.7.20
+
+No user-facing changes.
+
 ## 1.7.19
 
 No user-facing changes.

csharp/ql/campaigns/Solorigate/lib/change-notes/released/1.7.20.md

@@ -0,0 +1,3 @@
+## 1.7.20
+
+No user-facing changes.