Merge branch 'main' into scinit

Author: geoffw0

actions/ql/src/Security/CWE-829/UntrustedCheckoutCritical.md

@@ -1,6 +1,6 @@
 ## Overview
 
-GitHub workflows can be triggered through various repository events, including incoming pull requests (PRs) or comments on Issues/PRs. A potentially dangerous misuse of the triggers such as `pull_request_target` or `issue_comment` followed by an explicit checkout of untrusted code (Pull Request HEAD) may lead to repository compromise if untrusted code gets executed in a privileged job.
+GitHub workflows can be triggered through various repository events, including incoming pull requests (PRs) or comments on Issues/PRs. A potentially dangerous misuse of the triggers such as `pull_request_target` or `issue_comment` followed by an explicit checkout of untrusted code (Pull Request HEAD) may lead to repository compromise if untrusted code gets executed (e.g., due to a modified build script) in a privileged job.
 
 ## Recommendation
 
@@ -32,7 +32,7 @@ jobs:
 
       - uses: actions/setup-node@v1
       - run: |
-          npm install
+          npm install # scripts in package.json from PR would be executed here
           npm build
 
       - uses: completely/fakeaction@v2

actions/ql/src/Security/CWE-829/UntrustedCheckoutHigh.md

@@ -1,6 +1,6 @@
 ## Overview
 
-GitHub workflows can be triggered through various repository events, including incoming pull requests (PRs) or comments on Issues/PRs. A potentially dangerous misuse of the triggers such as `pull_request_target` or `issue_comment` followed by an explicit checkout of untrusted code (Pull Request HEAD) may lead to repository compromise if untrusted code gets executed in a privileged job.
+GitHub workflows can be triggered through various repository events, including incoming pull requests (PRs) or comments on Issues/PRs. A potentially dangerous misuse of the triggers such as `pull_request_target` or `issue_comment` followed by an explicit checkout of untrusted code (Pull Request HEAD) may lead to repository compromise if untrusted code gets executed (e.g., due to a modified build script) in a privileged job.
 
 ## Recommendation
 
@@ -32,7 +32,7 @@ jobs:
 
       - uses: actions/setup-node@v1
       - run: |
-          npm install
+          npm install # scripts in package.json from PR would be executed here
           npm build
 
       - uses: completely/fakeaction@v2

actions/ql/src/Security/CWE-829/UntrustedCheckoutMedium.md

@@ -1,6 +1,6 @@
 ## Overview
 
-GitHub workflows can be triggered through various repository events, including incoming pull requests (PRs) or comments on Issues/PRs. A potentially dangerous misuse of the triggers such as `pull_request_target` or `issue_comment` followed by an explicit checkout of untrusted code (Pull Request HEAD) may lead to repository compromise if untrusted code gets executed in a privileged job.
+GitHub workflows can be triggered through various repository events, including incoming pull requests (PRs) or comments on Issues/PRs. A potentially dangerous misuse of the triggers such as `pull_request_target` or `issue_comment` followed by an explicit checkout of untrusted code (Pull Request HEAD) may lead to repository compromise if untrusted code gets executed (e.g., due to a modified build script) in a privileged job.
 
 ## Recommendation
 
@@ -32,7 +32,7 @@ jobs:
 
       - uses: actions/setup-node@v1
       - run: |
-          npm install
+          npm install # scripts in package.json from PR would be executed here
           npm build
 
       - uses: completely/fakeaction@v2

cpp/ql/lib/change-notes/2025-08-02-gvn.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* The global value numbering library (`semmle.code.cpp.valuenumbering.GlobalValueNumbering` and `semmle.code.cpp.ir.ValueNumbering`) has been improved so more expressions are assigned the same value number.

cpp/ql/lib/change-notes/2025-08-11-global-variable-flow.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Improved dataflow through global variables in the new dataflow library (`semmle.code.cpp.dataflow.new.DataFlow` and `semmle.code.cpp.dataflow.new.TaintTracking`). Queries based on these libraries will produce more results on codebases with many global variables.

cpp/ql/lib/semmle/code/cpp/commons/Buffer.qll

@@ -57,19 +57,27 @@ private Class getRootType(FieldAccess fa) {
   )
 }
 
+/**
+ * Gets the size of `v`. This predicate does not have a result when the
+ * unspecified type of `v` is a `ReferenceType`.
+ */
+private int getVariableSize(Variable v) {
+  exists(Type t |
+    t = v.getUnspecifiedType() and
+    not t instanceof ReferenceType and
+    result = t.getSize()
+  )
+}
+
 /**
  * Gets the size of the buffer access at `va`.
  */
 private int getSize(VariableAccess va) {
   exists(Variable v | va.getTarget() = v |
     // If `v` is not a field then the size of the buffer is just
     // the size of the type of `v`.
-    exists(Type t |
-      t = v.getUnspecifiedType() and
-      not v instanceof Field and
-      not t instanceof ReferenceType and
-      result = t.getSize()
-    )
+    not v instanceof Field and
+    result = getVariableSize(v)
     or
     exists(Class c, int trueSize |
       // Otherwise, we find the "outermost" object and compute the size
@@ -92,7 +100,7 @@ private int getSize(VariableAccess va) {
       // buffer is `12 - 4 = 8`.
       c = getRootType(va) and
       // we calculate the size based on the last field, to avoid including any padding after it
-      trueSize = max(Field f | | f.getOffsetInClass(c) + f.getUnspecifiedType().getSize()) and
+      trueSize = max(Field f | | f.getOffsetInClass(c) + getVariableSize(f)) and
       result = trueSize - v.(Field).getOffsetInClass(c)
     )
   )

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

@@ -332,6 +332,13 @@ private module IndirectInstructions {
 
 import IndirectInstructions
 
+predicate isPostUpdateNodeImpl(Operand operand, int indirectionIndex) {
+  operand = any(FieldAddress fa).getObjectAddressOperand() and
+  indirectionIndex = [0 .. Ssa::countIndirectionsForCppType(Ssa::getLanguageType(operand))]
+  or
+  Ssa::isModifiableByCall(operand, indirectionIndex)
+}
+
 /** Gets the callable in which this node occurs. */
 DataFlowCallable nodeGetEnclosingCallable(Node n) { result = n.getEnclosingCallable() }
 

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

@@ -42,11 +42,7 @@ private newtype TIRDataFlowNode =
       [getMinIndirectionsForType(var.getUnspecifiedType()) .. SsaImpl::getMaxIndirectionsForType(var.getUnspecifiedType())]
   } or
   TPostUpdateNodeImpl(Operand operand, int indirectionIndex) {
-    operand = any(FieldAddress fa).getObjectAddressOperand() and
-    indirectionIndex =
-      [0 .. SsaImpl::countIndirectionsForCppType(SsaImpl::getLanguageType(operand))]
-    or
-    SsaImpl::isModifiableByCall(operand, indirectionIndex)
+    isPostUpdateNodeImpl(operand, indirectionIndex)
   } or
   TSsaSynthNode(SsaImpl::SynthNode n) or
   TSsaIteratorNode(IteratorFlow::IteratorFlowNode n) or

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

@@ -143,7 +143,14 @@ private predicate isGlobalUse(
     min(int cand, VariableAddressInstruction vai |
       vai.getEnclosingIRFunction() = f and
       vai.getAstVariable() = v and
-      isDef(_, _, _, vai, cand, indirectionIndex)
+      (
+        isDef(_, _, _, vai, cand, indirectionIndex)
+        or
+        exists(Operand operand |
+          isUse(_, operand, vai, cand, indirectionIndex) and
+          isPostUpdateNodeImpl(operand, indirectionIndex)
+        )
+      )
     |
       cand
     )

cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/gvn/internal/ValueNumberingInternal.qll

@@ -43,6 +43,23 @@ newtype TValueNumber =
   } or
   TUniqueValueNumber(IRFunction irFunc, Instruction instr) { uniqueValueNumber(instr, irFunc) }
 
+/**
+ * A `ConvertInstruction` which converts data of type `T` to data of type `U`
+ * where `T` and `U` only differ in specifiers. For example, if `T` is `int`
+ * and `U` is `const T` this is a conversion from a non-const integer to a
+ * const integer.
+ *
+ * Generally, the value number of a converted value is different from the value
+ * number of an unconverted value, but conversions which only modify specifiers
+ * leave the resulting value bitwise identical to the old value.
+ */
+class TypePreservingConvertInstruction extends ConvertInstruction {
+  TypePreservingConvertInstruction() {
+    pragma[only_bind_out](this.getResultType().getUnspecifiedType()) =
+      pragma[only_bind_out](this.getUnary().getResultType().getUnspecifiedType())
+  }
+}
+
 /**
  * A `CopyInstruction` whose source operand's value is congruent to the definition of that source
  * operand.
@@ -216,6 +233,7 @@ private predicate unaryValueNumber(
   not instr instanceof InheritanceConversionInstruction and
   not instr instanceof CopyInstruction and
   not instr instanceof FieldAddressInstruction and
+  not instr instanceof TypePreservingConvertInstruction and
   instr.getOpcode() = opcode and
   tvalueNumber(instr.getUnary()) = operand
 }
@@ -351,6 +369,10 @@ private TValueNumber nonUniqueValueNumber(Instruction instr) {
       or
       // The value number of a copy is just the value number of its source value.
       result = tvalueNumber(instr.(CongruentCopyInstruction).getSourceValue())
+      or
+      // The value number of a type-preserving conversion is just the value
+      // number of the unconverted value.
+      result = tvalueNumber(instr.(TypePreservingConvertInstruction).getUnary())
     )
   )
 }