Merge branch 'main' into redsun82/kotlin

.github/workflows/buildifier.yml

@@ -0,0 +1,28 @@
+name: Check bazel formatting
+
+on:
+  pull_request:
+    paths:
+      - "**.bazel"
+      - "**.bzl"
+    branches:
+      - main
+      - "rc/*"
+
+permissions:
+  contents: read
+
+jobs:
+  check:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+      - name: Check bazel formatting
+        uses: pre-commit/action@646c83fcd040023954eafda54b4db0192ce70507
+        with:
+          extra_args: >
+            buildifier --all-files 2>&1 ||
+            (
+              echo -e "In order to format all bazel files, please run:\n  bazel run //:buildifier"; exit 1
+            )

.pre-commit-config.yaml

@@ -20,13 +20,15 @@ repos:
       - id: autopep8
         files: ^misc/codegen/.*\.py
 
-  - repo: https://github.com/warchant/pre-commit-buildifier
-    rev: 0.0.2
-    hooks:
-      - id: buildifier
-
   - repo: local
     hooks:
+      - id: buildifier
+        name: Format bazel files
+        files: \.(bazel|bzl)
+        language: system
+        entry: bazel run //:buildifier
+        pass_filenames: false
+
       - id: codeql-format
         name: Fix QL file formatting
         files: \.qll?$

BUILD.bazel

@@ -0,0 +1,9 @@
+load("@buildifier_prebuilt//:rules.bzl", "buildifier")
+
+buildifier(
+    name = "buildifier",
+    exclude_patterns = [
+        "./.git/*",
+    ],
+    lint_mode = "fix",
+)

MODULE.bazel

@@ -23,6 +23,8 @@ bazel_dep(name = "nlohmann_json", version = "3.11.3", repo_name = "json")
 bazel_dep(name = "fmt", version = "10.0.0")
 bazel_dep(name = "rules_kotlin", version = "1.9.4-codeql.1")
 
+bazel_dep(name = "buildifier_prebuilt", version = "6.4.0", dev_dependency = True)
+
 pip = use_extension("@rules_python//python/extensions:pip.bzl", "pip")
 pip.parse(
     hub_name = "codegen_deps",

config/identical-files.json

@@ -362,7 +362,7 @@
     "java/ql/lib/semmle/code/java/security/internal/EncryptionKeySizes.qll"
   ],
   "Python model summaries test extension": [
-    "python/ql/test/experimental/dataflow/model-summaries/InlineTaintTest.ext.yml",
-    "python/ql/test/experimental/dataflow/model-summaries/NormalDataflowTest.ext.yml"
+    "python/ql/test/library-tests/dataflow/model-summaries/InlineTaintTest.ext.yml",
+    "python/ql/test/library-tests/dataflow/model-summaries/NormalDataflowTest.ext.yml"
   ]
-}
+}

cpp/ql/test/library-tests/ir/ir/ir.cpp

@@ -2432,7 +2432,7 @@ void initialization_with_temp_destructor() {
 }
 
 void param_with_destructor_by_value(ClassWithDestructor c) {
-    // The call to ~ClassWithDestructor::ClassWithDestructor() seems to be missing here.
+    // The call to ~ClassWithDestructor::ClassWithDestructor() happens on the side of the caller
 }
 
 void param_with_destructor_by_pointer(ClassWithDestructor* c) {

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

@@ -143,7 +143,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching
             // See https://docs.microsoft.com/en-us/dotnet/core/tools/global-json
             var versions = new List<string>();
 
-            foreach (var path in files.Where(p => p.EndsWith("global.json", StringComparison.Ordinal)))
+            foreach (var path in files.Where(p => string.Equals(FileUtils.SafeGetFileName(p, logger), "global.json", StringComparison.OrdinalIgnoreCase)))
             {
                 try
                 {

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

@@ -184,7 +184,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching
             {
                 try
                 {
-                    var isPackagesConfig = file.EndsWith("packages.config", StringComparison.OrdinalIgnoreCase);
+                    var isPackagesConfig = string.Equals(FileUtils.SafeGetFileName(file, logger), "packages.config", StringComparison.OrdinalIgnoreCase);
 
                     foreach (ReadOnlySpan<char> line in unsafeFileReader.ReadLines(file))
                     {

csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/SourceGenerators/DotnetSourceGeneratorBase.cs

@@ -55,7 +55,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching
 
             // group additional files by closes project file:
             var projects = fileProvider.Projects
-                .Select(p => (File: p, Directory: SafeGetDirectoryName(p)))
+                .Select(p => (File: p, Directory: FileUtils.SafeGetDirectoryName(p, logger)))
                 .Where(p => p.Directory.Length > 0);
 
             var groupedFiles = new Dictionary<string, List<string>>();
@@ -93,30 +93,6 @@ namespace Semmle.Extraction.CSharp.DependencyFetching
             }
         }
 
-        private string SafeGetDirectoryName(string fileName)
-        {
-            try
-            {
-                var dir = Path.GetDirectoryName(fileName);
-                if (dir is null)
-                {
-                    return "";
-                }
-
-                if (!dir.EndsWith(Path.DirectorySeparatorChar))
-                {
-                    dir += Path.DirectorySeparatorChar;
-                }
-
-                return dir;
-            }
-            catch (Exception ex)
-            {
-                logger.LogDebug($"Failed to get directory name for {fileName}: {ex.Message}");
-                return "";
-            }
-        }
-
         protected abstract ICollection<string> AdditionalFiles { get; }
 
         protected abstract string FileType { get; }

csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/SourceGenerators/ResxGenerator.cs

@@ -19,6 +19,13 @@ namespace Semmle.Extraction.CSharp.DependencyFetching
             TemporaryDirectory tempWorkingDirectory,
             IEnumerable<string> references) : base(fileProvider, fileContent, dotnet, compilationInfoContainer, logger, tempWorkingDirectory, references)
         {
+            if (fileProvider.Resources.Count == 0)
+            {
+                logger.LogDebug("No resources found, skipping resource extraction.");
+                sourceGeneratorFolder = null;
+                return;
+            }
+
             try
             {
                 // The package is downloaded to `missingpackages`, which is okay, we're already after the DLL collection phase.

csharp/extractor/Semmle.Util/FileUtils.cs

@@ -185,5 +185,42 @@ namespace Semmle.Util
 
             return new FileInfo(outputPath);
         }
+
+        public static string SafeGetDirectoryName(string path, ILogger logger)
+        {
+            try
+            {
+                var dir = Path.GetDirectoryName(path);
+                if (dir is null)
+                {
+                    return "";
+                }
+
+                if (!dir.EndsWith(Path.DirectorySeparatorChar))
+                {
+                    dir += Path.DirectorySeparatorChar;
+                }
+
+                return dir;
+            }
+            catch (Exception ex)
+            {
+                logger.LogDebug($"Failed to get directory name for {path}: {ex.Message}");
+                return "";
+            }
+        }
+
+        public static string? SafeGetFileName(string path, ILogger logger)
+        {
+            try
+            {
+                return Path.GetFileName(path);
+            }
+            catch (Exception ex)
+            {
+                logger.LogDebug($"Failed to get file name for {path}: {ex.Message}");
+                return null;
+            }
+        }
     }
 }

go/ql/src/Security/CWE-020/IncompleteHostnameRegexp.ql

@@ -81,14 +81,12 @@ predicate regexpGuardsError(RegexpPattern regexp) {
 
 module IncompleteHostNameRegexpConfig implements DataFlow::ConfigSig {
   additional predicate isSourceString(DataFlow::Node source, string hostPart) {
-    exists(Expr e |
-      e = source.asExpr() and
-      isIncompleteHostNameRegexpPattern(e.getStringValue(), hostPart)
-    |
-      e instanceof StringLit
-      or
-      e instanceof AddExpr and
-      not isIncompleteHostNameRegexpPattern(e.(AddExpr).getAnOperand().getStringValue(), _)
+    exists(Expr e | e = source.asExpr() |
+      isIncompleteHostNameRegexpPattern(e.getStringValue(), hostPart) and
+      // Exclude constant names to avoid duplicate results, because the string
+      // literals which they are initialised with are also considered as
+      // sources.
+      not e instanceof ConstantName
     )
   }
 
@@ -101,6 +99,10 @@ module IncompleteHostNameRegexpConfig implements DataFlow::ConfigSig {
     ) and
     not regexpGuardsError(sink)
   }
+
+  predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
+    StringOps::Concatenation::taintStep(node1, node2)
+  }
 }
 
 module Flow = DataFlow::Global<IncompleteHostNameRegexpConfig>;

go/ql/src/Security/CWE-020/IncompleteHostnameRegexpGood2.go

@@ -6,7 +6,7 @@ import (
 	"regexp"
 )
 
-func checkRedirectGood(req *http.Request, via []*http.Request) error {
+func checkRedirectGood2(req *http.Request, via []*http.Request) error {
 	// GOOD: the host of `req.URL` must be `example.com`, `www.example.com` or `beta.example.com`
 	re := `^((www|beta)\.)?example\.com/`
 	if matched, _ := regexp.MatchString(re, req.URL.Host); matched {

go/ql/src/change-notes/2024-04-24-incomplete-hostname-regexp.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* The query `go/incomplete-hostname-regexp` now recognizes more sources involving concatenation of string literals and also follows flow through string concatenation. This may lead to more alerts.

go/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegexp/IncompleteHostnameRegexp.expected

@@ -1,12 +1,22 @@
 edges
 | IncompleteHostnameRegexp.go:11:8:11:36 | "^((www\|beta).)?example.com/" | IncompleteHostnameRegexp.go:12:38:12:39 | re | provenance |  |
+| main.go:49:21:49:45 | `https://www.example.com` | main.go:62:15:62:25 | sourceConst | provenance |  |
+| main.go:62:15:62:25 | sourceConst | main.go:65:15:65:23 | localVar3 | provenance |  |
 nodes
 | IncompleteHostnameRegexp.go:11:8:11:36 | "^((www\|beta).)?example.com/" | semmle.label | "^((www\|beta).)?example.com/" |
 | IncompleteHostnameRegexp.go:12:38:12:39 | re | semmle.label | re |
-| main.go:39:60:39:79 | "^test2.github.com$" | semmle.label | "^test2.github.com$" |
-| main.go:44:15:44:39 | `https://www.example.com` | semmle.label | `https://www.example.com` |
+| main.go:40:60:40:79 | "^test2.github.com$" | semmle.label | "^test2.github.com$" |
+| main.go:45:15:45:39 | `https://www.example.com` | semmle.label | `https://www.example.com` |
+| main.go:49:21:49:45 | `https://www.example.com` | semmle.label | `https://www.example.com` |
+| main.go:56:15:56:34 | ...+... | semmle.label | ...+... |
+| main.go:58:15:58:42 | ...+... | semmle.label | ...+... |
+| main.go:62:15:62:25 | sourceConst | semmle.label | sourceConst |
+| main.go:65:15:65:23 | localVar3 | semmle.label | localVar3 |
 subpaths
 #select
 | IncompleteHostnameRegexp.go:11:8:11:36 | "^((www\|beta).)?example.com/" | IncompleteHostnameRegexp.go:11:8:11:36 | "^((www\|beta).)?example.com/" | IncompleteHostnameRegexp.go:12:38:12:39 | re | This regular expression has an unescaped dot before ')?example.com', so it might match more hosts than expected when $@. | IncompleteHostnameRegexp.go:12:38:12:39 | re | the regular expression is used |
-| main.go:39:60:39:79 | "^test2.github.com$" | main.go:39:60:39:79 | "^test2.github.com$" | main.go:39:60:39:79 | "^test2.github.com$" | This regular expression has an unescaped dot before 'github.com', so it might match more hosts than expected when $@. | main.go:39:60:39:79 | "^test2.github.com$" | the regular expression is used |
-| main.go:44:15:44:39 | `https://www.example.com` | main.go:44:15:44:39 | `https://www.example.com` | main.go:44:15:44:39 | `https://www.example.com` | This regular expression has an unescaped dot before 'example.com', so it might match more hosts than expected when $@. | main.go:44:15:44:39 | `https://www.example.com` | the regular expression is used |
+| main.go:40:60:40:79 | "^test2.github.com$" | main.go:40:60:40:79 | "^test2.github.com$" | main.go:40:60:40:79 | "^test2.github.com$" | This regular expression has an unescaped dot before 'github.com', so it might match more hosts than expected when $@. | main.go:40:60:40:79 | "^test2.github.com$" | the regular expression is used |
+| main.go:45:15:45:39 | `https://www.example.com` | main.go:45:15:45:39 | `https://www.example.com` | main.go:45:15:45:39 | `https://www.example.com` | This regular expression has an unescaped dot before 'example.com', so it might match more hosts than expected when $@. | main.go:45:15:45:39 | `https://www.example.com` | the regular expression is used |
+| main.go:49:21:49:45 | `https://www.example.com` | main.go:49:21:49:45 | `https://www.example.com` | main.go:65:15:65:23 | localVar3 | This regular expression has an unescaped dot before 'example.com', so it might match more hosts than expected when $@. | main.go:65:15:65:23 | localVar3 | the regular expression is used |
+| main.go:56:15:56:34 | ...+... | main.go:56:15:56:34 | ...+... | main.go:56:15:56:34 | ...+... | This regular expression has an unescaped dot before 'example.com', so it might match more hosts than expected when $@. | main.go:56:15:56:34 | ...+... | the regular expression is used |
+| main.go:58:15:58:42 | ...+... | main.go:58:15:58:42 | ...+... | main.go:58:15:58:42 | ...+... | This regular expression has an unescaped dot before 'example.com', so it might match more hosts than expected when $@. | main.go:58:15:58:42 | ...+... | the regular expression is used |

go/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegexp/IncompleteHostnameRegexpGood2.go

@@ -0,0 +1,16 @@
+package main
+
+import (
+	"errors"
+	"net/http"
+	"regexp"
+)
+
+func checkRedirectGood2(req *http.Request, via []*http.Request) error {
+	// GOOD: the host of `req.URL` must be `example.com`, `www.example.com` or `beta.example.com`
+	re := `^((www|beta)\.)?example\.com/`
+	if matched, _ := regexp.MatchString(re, req.URL.Host); matched {
+		return nil
+	}
+	return errors.New("Invalid redirect")
+}

go/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegexp/main.go

@@ -3,10 +3,11 @@
 package main
 
 import (
-	"github.com/elazarl/goproxy"
 	"net/http"
 	"regexp"
 	"time"
+
+	"github.com/elazarl/goproxy"
 )
 
 func Match(notARegex string) bool {
@@ -44,3 +45,22 @@ func main() {
 	regexp.Match(`https://www.example.com`, []byte(""))   // NOT OK
 	regexp.Match(`https://www\.example\.com`, []byte("")) // OK
 }
+
+const sourceConst = `https://www.example.com`
+const firstHalfConst = `https://www.example.`
+
+func concatenateStrings() {
+	firstHalf := `https://www.example.`
+	regexp.Match(firstHalf+`com`, []byte("")) // MISSING: NOT OK
+
+	regexp.Match(firstHalfConst+`com`, []byte("")) // NOT OK
+
+	regexp.Match(`https://www.example.`+`com`, []byte("")) // NOT OK
+}
+
+func avoidDuplicateResults() {
+	localVar1 := sourceConst
+	localVar2 := localVar1
+	localVar3 := localVar2
+	regexp.Match(localVar3, []byte("")) // NOT OK
+}

javascript/BUILD.bazel

@@ -1,5 +1,5 @@
-load("@semmle_code//:dist.bzl", "dist")
 load("@rules_pkg//pkg:mappings.bzl", "pkg_files")
+load("@semmle_code//:dist.bzl", "dist")
 load("@semmle_code//buildutils-internal:zipmerge.bzl", "zipmerge")
 
 package(default_visibility = ["//visibility:public"])

javascript/extractor/BUILD.bazel

@@ -1,5 +1,5 @@
-load("@semmle_code//:common.bzl", "codeql_fat_jar", "codeql_java_project")
 load("@rules_pkg//pkg:mappings.bzl", "pkg_files")
+load("@semmle_code//:common.bzl", "codeql_fat_jar", "codeql_java_project")
 
 java_library(
     name = "deps",

misc/codegen/BUILD.bazel

@@ -1,5 +1,3 @@
-load("@codegen_deps//:requirements.bzl", "requirement")
-
 py_binary(
     name = "codegen",
     srcs = ["codegen.py"],

misc/codegen/generators/BUILD.bazel

@@ -1,5 +1,3 @@
-load("@codegen_deps//:requirements.bzl", "requirement")
-
 py_library(
     name = "generators",
     srcs = glob(["*.py"]),

python/extractor/tsg-python/tsp/BUILD.bazel

@@ -1,6 +1,6 @@
+load("@py_deps//:defs.bzl", "aliases", "all_crate_deps")
 load("@rules_rust//cargo:defs.bzl", "cargo_build_script")
 load("@rules_rust//rust:defs.bzl", "rust_library")
-load("@py_deps//:defs.bzl", "aliases", "all_crate_deps")
 
 package(default_visibility = ["//visibility:public"])
 

python/ql/lib/semmle/python/dataflow/new/SensitiveDataSources.qll

@@ -89,7 +89,7 @@ private module SensitiveDataModeling {
    */
   DataFlow::Node sensitiveLookupStringConst(SensitiveDataClassification classification) {
     // Note: If this is implemented with type-tracking, we will get cross-talk as
-    // illustrated in python/ql/test/experimental/dataflow/sensitive-data/test.py
+    // illustrated in python/ql/test/library-tests/dataflow/sensitive-data/test.py
     exists(DataFlow::LocalSourceNode source |
       source.asExpr().(StringLiteral).getText() = sensitiveString(classification) and
       source.flowsTo(result)

python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPublic.qll

@@ -638,7 +638,7 @@ newtype TContent =
     //   name = any(AccessPathToken a).getAnArgument("Attribute")
     // instead we use a qltest to alert if we write a new summary in QL that uses an
     // attribute -- see
-    // python/ql/test/experimental/dataflow/summaries-checks/missing-attribute-content.ql
+    // python/ql/test/library-tests/dataflow/summaries-checks/missing-attribute-content.ql
     attr in ["re", "string", "pattern"]
     or
     //

python/ql/test/TestUtilities/dataflow/NormalDataflowTest.qll

@@ -1,6 +1,6 @@
 import python
-import experimental.dataflow.TestUtil.FlowTest
-import experimental.dataflow.testConfig
+import TestUtilities.dataflow.FlowTest
+import TestUtilities.dataflow.testConfig
 private import semmle.python.dataflow.new.internal.PrintNode
 
 module DataFlowTest implements FlowTestSig {

python/ql/test/TestUtilities/dataflow/NormalTaintTrackingTest.qll

@@ -1,6 +1,6 @@
 import python
-import experimental.dataflow.TestUtil.FlowTest
-import experimental.dataflow.testTaintConfig
+import TestUtilities.dataflow.FlowTest
+import TestUtilities.dataflow.testTaintConfig
 private import semmle.python.dataflow.new.internal.PrintNode
 
 module DataFlowTest implements FlowTestSig {

python/ql/test/experimental/dataflow/basic/localFlowStepTest.ql

@@ -1 +0,0 @@
-import experimental.dataflow.TestUtil.LocalFlowStepTest

python/ql/test/experimental/dataflow/basic/maximalFlowTest.ql

@@ -1 +0,0 @@
-import experimental.dataflow.TestUtil.MaximalFlowTest

python/ql/test/experimental/dataflow/coverage/NormalDataflowTest.ql

@@ -1,2 +0,0 @@
-import python
-import experimental.dataflow.TestUtil.NormalDataflowTest

python/ql/test/experimental/dataflow/exceptions/NormalDataflowTest.ql

@@ -1,2 +0,0 @@
-import python
-import experimental.dataflow.TestUtil.NormalDataflowTest

python/ql/test/experimental/dataflow/fieldflow/NormalDataflowTest.ql

@@ -1,2 +0,0 @@
-import python
-import experimental.dataflow.TestUtil.NormalDataflowTest

python/ql/test/experimental/dataflow/match/NormalDataflowTest.ql

@@ -1,2 +0,0 @@
-import python
-import experimental.dataflow.TestUtil.NormalDataflowTest

python/ql/test/experimental/dataflow/model-summaries/NormalDataflowTest.ql

@@ -1,2 +0,0 @@
-import python
-import experimental.dataflow.TestUtil.NormalDataflowTest

python/ql/test/experimental/dataflow/summaries/NormalTaintTrackingTest.ql

@@ -1,3 +0,0 @@
-import python
-private import TestSummaries
-import experimental.dataflow.TestUtil.NormalTaintTrackingTest

python/ql/test/experimental/dataflow/tainttracking/generator-flow/NormalDataflowTest.ql

@@ -1,2 +0,0 @@
-import python
-import experimental.dataflow.TestUtil.NormalDataflowTest

python/ql/test/experimental/meta/debug/dataflowTestPaths.ql

@@ -9,7 +9,7 @@
 // 3. if necessary, look at partial paths by (un)commenting appropriate lines
 import python
 import semmle.python.dataflow.new.DataFlow
-import experimental.dataflow.testConfig
+import TestUtilities.dataflow.testConfig
 
 module Config implements DataFlow::ConfigSig {
   predicate isSource(DataFlow::Node source) { TestConfig::isSource(source) }

python/ql/test/experimental/query-tests/Security/CWE-022-UnsafeUnpacking/DataflowQueryTest.ql

@@ -1,4 +1,4 @@
 import python
-import experimental.dataflow.TestUtil.DataflowQueryTest
+import TestUtilities.dataflow.DataflowQueryTest
 import experimental.Security.UnsafeUnpackQuery
 import FromTaintTrackingConfig<UnsafeUnpackConfig>

python/ql/test/experimental/query-tests/Security/CWE-409/DataflowQueryTest.ql

@@ -1,4 +1,4 @@
 import python
-import experimental.dataflow.TestUtil.DataflowQueryTest
+import TestUtilities.dataflow.DataflowQueryTest
 import experimental.semmle.python.security.DecompressionBomb
 import FromTaintTrackingConfig<BombsConfig>

python/ql/test/library-tests/dataflow/basic/callGraph.ql

@@ -1,4 +1,4 @@
-import experimental.dataflow.callGraphConfig
+import TestUtilities.dataflow.callGraphConfig
 
 from DataFlow::Node source, DataFlow::Node sink
 where

python/ql/test/library-tests/dataflow/basic/callGraphSinks.ql

@@ -1,4 +1,4 @@
-import experimental.dataflow.callGraphConfig
+import TestUtilities.dataflow.callGraphConfig
 
 from DataFlow::Node sink
 where

python/ql/test/library-tests/dataflow/basic/callGraphSources.ql

@@ -1,4 +1,4 @@
-import experimental.dataflow.callGraphConfig
+import TestUtilities.dataflow.callGraphConfig
 
 from DataFlow::Node source
 where

python/ql/test/library-tests/dataflow/basic/localFlowStepTest.ql

@@ -0,0 +1 @@
+import TestUtilities.dataflow.LocalFlowStepTest

python/ql/test/library-tests/dataflow/basic/maximalFlowTest.ql

@@ -0,0 +1 @@
+import TestUtilities.dataflow.MaximalFlowTest

python/ql/test/library-tests/dataflow/coverage/NormalDataflowTest.ql

@@ -0,0 +1,2 @@
+import python
+import TestUtilities.dataflow.NormalDataflowTest