Merge branch 'main' into criemen/rust-bzlmod-new

cpp/ql/lib/change-notes/2024-10-06-builtin-expect.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* 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/semmle/code/cpp/controlflow/IRGuards.qll

@@ -762,6 +762,8 @@ private predicate compares_eq(
   exists(AbstractValue dual | value = dual.getDualValue() |
     compares_eq(test.(LogicalNotInstruction).getUnary(), left, right, k, areEqual, dual)
   )
+  or
+  compares_eq(test.(BuiltinExpectCallInstruction).getCondition(), left, right, k, areEqual, value)
 }
 
 /**
@@ -831,6 +833,9 @@ private predicate unary_compares_eq(
     int_value(const) = k1 and
     k = k1 + k2
   )
+  or
+  unary_compares_eq(test.(BuiltinExpectCallInstruction).getCondition(), op, k, areEqual,
+    inNonZeroCase, value)
 }
 
 /** Rearrange various simple comparisons into `left == right + k` form. */
@@ -910,12 +915,68 @@ private predicate unary_simple_comparison_eq(
   )
 }
 
+/** A call to the builtin operation `__builtin_expect`. */
+private class BuiltinExpectCallInstruction extends CallInstruction {
+  BuiltinExpectCallInstruction() { this.getStaticCallTarget().hasName("__builtin_expect") }
+
+  /** Gets the condition of this call. */
+  Instruction getCondition() {
+    // The first parameter of `__builtin_expect` has type `long`. So we skip
+    // the conversion when inferring guards.
+    result = this.getArgument(0).(ConvertInstruction).getUnary()
+  }
+}
+
+/**
+ * Holds if `left == right + k` is `areEqual` if `cmp` evaluates to `value`,
+ * and `cmp` is an instruction that compares the value of
+ * `__builtin_expect(left == right + k, _)` to `0`.
+ */
+private predicate builtin_expect_eq(
+  CompareInstruction cmp, Operand left, Operand right, int k, boolean areEqual, AbstractValue value
+) {
+  exists(BuiltinExpectCallInstruction call, Instruction const, AbstractValue innerValue |
+    int_value(const) = 0 and
+    cmp.hasOperands(call.getAUse(), const.getAUse()) and
+    compares_eq(call.getCondition(), left, right, k, areEqual, innerValue)
+  |
+    cmp instanceof CompareNEInstruction and
+    value = innerValue
+    or
+    cmp instanceof CompareEQInstruction and
+    value.getDualValue() = innerValue
+  )
+}
+
 private predicate complex_eq(
   CompareInstruction cmp, Operand left, Operand right, int k, boolean areEqual, AbstractValue value
 ) {
   sub_eq(cmp, left, right, k, areEqual, value)
   or
   add_eq(cmp, left, right, k, areEqual, value)
+  or
+  builtin_expect_eq(cmp, left, right, k, areEqual, value)
+}
+
+/**
+ * Holds if `op == k` is `areEqual` if `cmp` evaluates to `value`, and `cmp` is
+ * an instruction that compares the value of `__builtin_expect(op == k, _)` to `0`.
+ */
+private predicate unary_builtin_expect_eq(
+  CompareInstruction cmp, Operand op, int k, boolean areEqual, boolean inNonZeroCase,
+  AbstractValue value
+) {
+  exists(BuiltinExpectCallInstruction call, Instruction const, AbstractValue innerValue |
+    int_value(const) = 0 and
+    cmp.hasOperands(call.getAUse(), const.getAUse()) and
+    unary_compares_eq(call.getCondition(), op, k, areEqual, inNonZeroCase, innerValue)
+  |
+    cmp instanceof CompareNEInstruction and
+    value = innerValue
+    or
+    cmp instanceof CompareEQInstruction and
+    value.getDualValue() = innerValue
+  )
 }
 
 private predicate unary_complex_eq(
@@ -924,6 +985,8 @@ private predicate unary_complex_eq(
   unary_sub_eq(test, op, k, areEqual, inNonZeroCase, value)
   or
   unary_add_eq(test, op, k, areEqual, inNonZeroCase, value)
+  or
+  unary_builtin_expect_eq(test, op, k, areEqual, inNonZeroCase, value)
 }
 
 /*

cpp/ql/test/library-tests/controlflow/guards/Guards.expected

@@ -45,3 +45,7 @@
 | test.cpp:122:9:122:9 | b |
 | test.cpp:125:13:125:20 | ! ... |
 | test.cpp:125:14:125:17 | call to safe |
+| test.cpp:131:6:131:21 | call to __builtin_expect |
+| test.cpp:135:6:135:21 | call to __builtin_expect |
+| test.cpp:141:6:141:21 | call to __builtin_expect |
+| test.cpp:145:6:145:21 | call to __builtin_expect |

cpp/ql/test/library-tests/controlflow/guards/GuardsCompare.expected

@@ -164,10 +164,46 @@
 | 126 | call to test3_condition != 0 when ... && ... is true |
 | 126 | call to test3_condition != 0 when call to test3_condition is true |
 | 126 | call to test3_condition == 0 when call to test3_condition is false |
+| 131 | ... + ... != a+0 when call to __builtin_expect is false |
+| 131 | ... + ... == a+0 when call to __builtin_expect is true |
+| 131 | a != ... + ...+0 when call to __builtin_expect is false |
+| 131 | a != b+42 when call to __builtin_expect is false |
+| 131 | a == ... + ...+0 when call to __builtin_expect is true |
+| 131 | a == b+42 when call to __builtin_expect is true |
 | 131 | b != 0 when b is true |
+| 131 | b != a+-42 when call to __builtin_expect is false |
 | 131 | b == 0 when b is false |
+| 131 | b == a+-42 when call to __builtin_expect is true |
+| 131 | call to __builtin_expect != 0 when call to __builtin_expect is true |
+| 131 | call to __builtin_expect == 0 when call to __builtin_expect is false |
+| 135 | ... + ... != a+0 when call to __builtin_expect is true |
+| 135 | ... + ... == a+0 when call to __builtin_expect is false |
+| 135 | a != ... + ...+0 when call to __builtin_expect is true |
+| 135 | a != b+42 when call to __builtin_expect is true |
+| 135 | a == ... + ...+0 when call to __builtin_expect is false |
+| 135 | a == b+42 when call to __builtin_expect is false |
+| 135 | b != a+-42 when call to __builtin_expect is true |
+| 135 | b == a+-42 when call to __builtin_expect is false |
+| 135 | call to __builtin_expect != 0 when call to __builtin_expect is true |
+| 135 | call to __builtin_expect == 0 when call to __builtin_expect is false |
 | 137 | 0 != 0 when 0 is true |
 | 137 | 0 == 0 when 0 is false |
+| 141 | 42 != a+0 when call to __builtin_expect is false |
+| 141 | 42 == a+0 when call to __builtin_expect is true |
+| 141 | a != 42 when call to __builtin_expect is false |
+| 141 | a != 42+0 when call to __builtin_expect is false |
+| 141 | a == 42 when call to __builtin_expect is true |
+| 141 | a == 42+0 when call to __builtin_expect is true |
+| 141 | call to __builtin_expect != 0 when call to __builtin_expect is true |
+| 141 | call to __builtin_expect == 0 when call to __builtin_expect is false |
+| 145 | 42 != a+0 when call to __builtin_expect is true |
+| 145 | 42 == a+0 when call to __builtin_expect is false |
+| 145 | a != 42 when call to __builtin_expect is true |
+| 145 | a != 42+0 when call to __builtin_expect is true |
+| 145 | a == 42 when call to __builtin_expect is false |
+| 145 | a == 42+0 when call to __builtin_expect is false |
+| 145 | call to __builtin_expect != 0 when call to __builtin_expect is true |
+| 145 | call to __builtin_expect == 0 when call to __builtin_expect is false |
 | 146 | ! ... != 0 when ! ... is true |
 | 146 | ! ... == 0 when ! ... is false |
 | 146 | x != 0 when ! ... is false |

cpp/ql/test/library-tests/controlflow/guards/GuardsControl.expected

@@ -104,3 +104,7 @@
 | test.cpp:122:9:122:9 | b | true | 125 | 125 |
 | test.cpp:125:13:125:20 | ! ... | true | 125 | 125 |
 | test.cpp:125:14:125:17 | call to safe | false | 125 | 125 |
+| test.cpp:131:6:131:21 | call to __builtin_expect | true | 131 | 132 |
+| test.cpp:135:6:135:21 | call to __builtin_expect | true | 135 | 136 |
+| test.cpp:141:6:141:21 | call to __builtin_expect | true | 141 | 142 |
+| test.cpp:145:6:145:21 | call to __builtin_expect | true | 145 | 146 |

cpp/ql/test/library-tests/controlflow/guards/GuardsEnsure.expected

@@ -159,6 +159,18 @@ binary
 | test.cpp:105:6:105:14 | ... != ... | test.cpp:105:11:105:14 | 0.0 | != | test.cpp:105:6:105:6 | f | 0 | 105 | 106 |
 | test.cpp:111:6:111:14 | ... != ... | test.cpp:111:6:111:6 | i | != | test.cpp:111:11:111:14 | 0.0 | 0 | 111 | 112 |
 | test.cpp:111:6:111:14 | ... != ... | test.cpp:111:11:111:14 | 0.0 | != | test.cpp:111:6:111:6 | i | 0 | 111 | 112 |
+| test.cpp:131:6:131:21 | call to __builtin_expect | test.cpp:131:23:131:23 | a | == | test.cpp:131:28:131:28 | b | 42 | 131 | 132 |
+| test.cpp:131:6:131:21 | call to __builtin_expect | test.cpp:131:23:131:23 | a | == | test.cpp:131:28:131:33 | ... + ... | 0 | 131 | 132 |
+| test.cpp:131:6:131:21 | call to __builtin_expect | test.cpp:131:28:131:28 | b | == | test.cpp:131:23:131:23 | a | -42 | 131 | 132 |
+| test.cpp:131:6:131:21 | call to __builtin_expect | test.cpp:131:28:131:33 | ... + ... | == | test.cpp:131:23:131:23 | a | 0 | 131 | 132 |
+| test.cpp:135:6:135:21 | call to __builtin_expect | test.cpp:135:23:135:23 | a | != | test.cpp:135:28:135:28 | b | 42 | 135 | 136 |
+| test.cpp:135:6:135:21 | call to __builtin_expect | test.cpp:135:23:135:23 | a | != | test.cpp:135:28:135:33 | ... + ... | 0 | 135 | 136 |
+| test.cpp:135:6:135:21 | call to __builtin_expect | test.cpp:135:28:135:28 | b | != | test.cpp:135:23:135:23 | a | -42 | 135 | 136 |
+| test.cpp:135:6:135:21 | call to __builtin_expect | test.cpp:135:28:135:33 | ... + ... | != | test.cpp:135:23:135:23 | a | 0 | 135 | 136 |
+| test.cpp:141:6:141:21 | call to __builtin_expect | test.cpp:141:23:141:23 | a | == | test.cpp:141:28:141:29 | 42 | 0 | 141 | 142 |
+| test.cpp:141:6:141:21 | call to __builtin_expect | test.cpp:141:28:141:29 | 42 | == | test.cpp:141:23:141:23 | a | 0 | 141 | 142 |
+| test.cpp:145:6:145:21 | call to __builtin_expect | test.cpp:145:23:145:23 | a | != | test.cpp:145:28:145:29 | 42 | 0 | 145 | 146 |
+| test.cpp:145:6:145:21 | call to __builtin_expect | test.cpp:145:28:145:29 | 42 | != | test.cpp:145:23:145:23 | a | 0 | 145 | 146 |
 unary
 | test.c:7:9:7:13 | ... > ... | test.c:7:9:7:9 | x | < | 1 | 10 | 11 |
 | test.c:7:9:7:13 | ... > ... | test.c:7:9:7:9 | x | >= | 1 | 7 | 9 |
@@ -270,3 +282,9 @@ unary
 | test.cpp:122:9:122:9 | b | test.cpp:122:9:122:9 | b | != | 0 | 125 | 125 |
 | test.cpp:125:13:125:20 | ! ... | test.cpp:125:13:125:20 | ! ... | != | 0 | 125 | 125 |
 | test.cpp:125:14:125:17 | call to safe | test.cpp:125:14:125:17 | call to safe | == | 0 | 125 | 125 |
+| test.cpp:131:6:131:21 | call to __builtin_expect | test.cpp:131:6:131:21 | call to __builtin_expect | != | 0 | 131 | 132 |
+| test.cpp:135:6:135:21 | call to __builtin_expect | test.cpp:135:6:135:21 | call to __builtin_expect | != | 0 | 135 | 136 |
+| test.cpp:141:6:141:21 | call to __builtin_expect | test.cpp:141:6:141:21 | call to __builtin_expect | != | 0 | 141 | 142 |
+| test.cpp:141:6:141:21 | call to __builtin_expect | test.cpp:141:23:141:23 | a | == | 42 | 141 | 142 |
+| test.cpp:145:6:145:21 | call to __builtin_expect | test.cpp:145:6:145:21 | call to __builtin_expect | != | 0 | 145 | 146 |
+| test.cpp:145:6:145:21 | call to __builtin_expect | test.cpp:145:23:145:23 | a | != | 42 | 145 | 146 |

cpp/ql/test/library-tests/controlflow/guards/test.cpp

@@ -125,4 +125,24 @@ void test(bool b)
         if (!safe(x)) return;
     }
     use(x);
+}
+
+void binary_test_builtin_expected(int a, int b) {
+  if(__builtin_expect(a == b + 42, 0)) {
+      use(a);
+  }
+
+  if(__builtin_expect(a != b + 42, 0)) {
+      use(a);
+  }
+}
+
+void unary_test_builtin_expected(int a) {
+  if(__builtin_expect(a == 42, 0)) {
+      use(a);
+  }
+
+  if(__builtin_expect(a != 42, 0)) {
+      use(a);
+  }
 }

go/extractor/project/project.go

@@ -36,6 +36,17 @@ type GoModule struct {
 	Module *modfile.File // The parsed contents of the `go.mod` file
 }
 
+// Tries to find the Go toolchain version required for this module.
+func (module *GoModule) RequiredGoVersion() util.SemVer {
+	if module.Module != nil && module.Module.Toolchain != nil {
+		return util.NewSemVer(module.Module.Toolchain.Name)
+	} else if module.Module != nil && module.Module.Go != nil {
+		return util.NewSemVer(module.Module.Go.Version)
+	} else {
+		return tryReadGoDirective(module.Path)
+	}
+}
+
 // Represents information about a Go project workspace: this may either be a folder containing
 // a `go.work` file or a collection of `go.mod` files.
 type GoWorkspace struct {
@@ -54,24 +65,23 @@ type GoVersionInfo = util.SemVer
 // 1. The Go version specified in the `go.work` file, if any.
 // 2. The greatest Go version specified in any `go.mod` file, if any.
 func (workspace *GoWorkspace) RequiredGoVersion() util.SemVer {
-	if workspace.WorkspaceFile != nil && workspace.WorkspaceFile.Go != nil {
-		// If we have parsed a `go.work` file, return the version number from it.
+	// If we have parsed a `go.work` file, we prioritise versions from it over those in individual `go.mod`
+	// files. We are interested in toolchain versions, so if there is an explicit toolchain declaration in
+	// a `go.work` file, we use that. Otherwise, we fall back to the language version in the `go.work` file
+	// and use that as toolchain version. If we didn't parse a `go.work` file, then we try to find the
+	// greatest version contained in `go.mod` files.
+	if workspace.WorkspaceFile != nil && workspace.WorkspaceFile.Toolchain != nil {
+		return util.NewSemVer(workspace.WorkspaceFile.Toolchain.Name)
+	} else if workspace.WorkspaceFile != nil && workspace.WorkspaceFile.Go != nil {
 		return util.NewSemVer(workspace.WorkspaceFile.Go.Version)
 	} else if workspace.Modules != nil && len(workspace.Modules) > 0 {
 		// Otherwise, if we have `go.work` files, find the greatest Go version in those.
 		var greatestVersion util.SemVer = nil
 		for _, module := range workspace.Modules {
-			if module.Module != nil && module.Module.Go != nil {
-				// If we have parsed the file, retrieve the version number we have already obtained.
-				modVersion := util.NewSemVer(module.Module.Go.Version)
-				if greatestVersion == nil || modVersion.IsNewerThan(greatestVersion) {
-					greatestVersion = modVersion
-				}
-			} else {
-				modVersion := tryReadGoDirective(module.Path)
-				if modVersion != nil && (greatestVersion == nil || modVersion.IsNewerThan(greatestVersion)) {
-					greatestVersion = modVersion
-				}
+			modVersion := module.RequiredGoVersion()
+
+			if modVersion != nil && (greatestVersion == nil || modVersion.IsNewerThan(greatestVersion)) {
+				greatestVersion = modVersion
 			}
 		}
 

go/extractor/project/project_test.go

@@ -4,6 +4,7 @@ import (
 	"path/filepath"
 	"testing"
 
+	"github.com/github/codeql-go/extractor/util"
 	"golang.org/x/mod/modfile"
 )
 
@@ -28,14 +29,18 @@ func TestStartsWithAnyOf(t *testing.T) {
 	testStartsWithAnyOf(t, filepath.Join("foo", "bar"), filepath.Join("foo", "baz"), false)
 }
 
-func testHasInvalidToolchainVersion(t *testing.T, contents string) bool {
-	modFile, err := modfile.Parse("test.go", []byte(contents), nil)
+func parseModFile(t *testing.T, contents string) *modfile.File {
+	modFile, err := modfile.Parse("go.mod", []byte(contents), nil)
 
 	if err != nil {
 		t.Errorf("Unable to parse %s: %s.\n", contents, err.Error())
 	}
 
-	return hasInvalidToolchainVersion(modFile)
+	return modFile
+}
+
+func testHasInvalidToolchainVersion(t *testing.T, contents string) bool {
+	return hasInvalidToolchainVersion(parseModFile(t, contents))
 }
 
 func TestHasInvalidToolchainVersion(t *testing.T) {
@@ -62,3 +67,74 @@ func TestHasInvalidToolchainVersion(t *testing.T) {
 		}
 	}
 }
+
+func parseWorkFile(t *testing.T, contents string) *modfile.WorkFile {
+	workFile, err := modfile.ParseWork("go.work", []byte(contents), nil)
+
+	if err != nil {
+		t.Errorf("Unable to parse %s: %s.\n", contents, err.Error())
+	}
+
+	return workFile
+}
+
+func TestRequiredGoVersion(t *testing.T) {
+	type ModVersionPair struct {
+		FileContents    string
+		ExpectedVersion string
+	}
+
+	modules := []ModVersionPair{
+		{"go 1.20", "v1.20"},
+		{"go 1.21.2", "v1.21.2"},
+		{"go 1.21rc1", "v1.21.0-rc1"},
+		{"go 1.21rc1\ntoolchain go1.22.0", "v1.22.0"},
+		{"go 1.21rc1\ntoolchain go1.22rc1", "v1.22.0-rc1"},
+	}
+
+	for _, testData := range modules {
+		// `go.mod` and `go.work` files have mostly the same format
+		modFile := parseModFile(t, testData.FileContents)
+		workFile := parseWorkFile(t, testData.FileContents)
+		mod := GoModule{
+			Path:   "test", // irrelevant
+			Module: modFile,
+		}
+		work := GoWorkspace{
+			WorkspaceFile: workFile,
+		}
+
+		result := mod.RequiredGoVersion()
+		if result == nil {
+			t.Errorf(
+				"Expected mod.RequiredGoVersion() to return %s for the below `go.mod` file, but got nothing:\n%s",
+				testData.ExpectedVersion,
+				testData.FileContents,
+			)
+		} else if result != util.NewSemVer(testData.ExpectedVersion) {
+			t.Errorf(
+				"Expected mod.RequiredGoVersion() to return %s for the below `go.mod` file, but got %s:\n%s",
+				testData.ExpectedVersion,
+				result,
+				testData.FileContents,
+			)
+		}
+
+		result = work.RequiredGoVersion()
+		if result == nil {
+			t.Errorf(
+				"Expected mod.RequiredGoVersion() to return %s for the below `go.work` file, but got nothing:\n%s",
+				testData.ExpectedVersion,
+				testData.FileContents,
+			)
+		} else if result != util.NewSemVer(testData.ExpectedVersion) {
+			t.Errorf(
+				"Expected mod.RequiredGoVersion() to return %s for the below `go.work` file, but got %s:\n%s",
+				testData.ExpectedVersion,
+				result,
+				testData.FileContents,
+			)
+		}
+	}
+
+}

java/ql/lib/semmle/code/java/security/BrokenCryptoAlgorithmQuery.qll

@@ -15,6 +15,8 @@ private class ShortStringLiteral extends StringLiteral {
 class BrokenAlgoLiteral extends ShortStringLiteral {
   BrokenAlgoLiteral() {
     this.getValue().regexpMatch(getInsecureAlgorithmRegex()) and
+    // Exclude RSA/ECB/.* ciphers.
+    not this.getValue().regexpMatch("RSA/ECB.*") and
     // Exclude German and French sentences.
     not this.getValue().regexpMatch(".*\\p{IsLowercase} des \\p{IsLetter}.*")
   }