Merge branch 'main' into calumgrant/bmn/wrong-type-format-arg-linkage

cpp/ql/src/Security/CWE/CWE-190/TaintedAllocationSize.ql

@@ -33,8 +33,9 @@ predicate allocSink(HeuristicAllocationExpr alloc, DataFlow::Node sink) {
   )
 }
 
-predicate readsVariable(LoadInstruction load, Variable var) {
-  load.getSourceAddress().(VariableAddressInstruction).getAstVariable() = var
+predicate readsVariable(LoadInstruction load, Variable var, IRBlock bb) {
+  load.getSourceAddress().(VariableAddressInstruction).getAstVariable() = var and
+  bb = load.getBlock()
 }
 
 predicate hasUpperBoundsCheck(Variable var) {
@@ -46,10 +47,18 @@ predicate hasUpperBoundsCheck(Variable var) {
   )
 }
 
-predicate nodeIsBarrierEqualityCandidate(DataFlow::Node node, Operand access, Variable checkedVar) {
-  exists(Instruction instr | instr = node.asOperand().getDef() |
-    readsVariable(instr, checkedVar) and
-    any(IRGuardCondition guard).ensuresEq(access, _, _, instr.getBlock(), true)
+predicate variableEqualityCheckedInBlock(Variable checkedVar, IRBlock bb) {
+  exists(Operand access |
+    readsVariable(access.getDef(), checkedVar, _) and
+    any(IRGuardCondition guard).ensuresEq(access, _, _, bb, true)
+  )
+}
+
+predicate nodeIsBarrierEquality(DataFlow::Node node) {
+  exists(Variable checkedVar, Instruction instr, IRBlock bb |
+    instr = node.asOperand().getDef() and
+    readsVariable(instr, checkedVar, bb) and
+    variableEqualityCheckedInBlock(checkedVar, bb)
   )
 }
 
@@ -72,14 +81,11 @@ module TaintedAllocationSizeConfig implements DataFlow::ConfigSig {
     )
     or
     exists(Variable checkedVar, Instruction instr | instr = node.asOperand().getDef() |
-      readsVariable(instr, checkedVar) and
+      readsVariable(instr, checkedVar, _) and
       hasUpperBoundsCheck(checkedVar)
     )
     or
-    exists(Variable checkedVar, Operand access |
-      readsVariable(access.getDef(), checkedVar) and
-      nodeIsBarrierEqualityCandidate(node, access, checkedVar)
-    )
+    nodeIsBarrierEquality(node)
     or
     // block flow to inside of identified allocation functions (this flow leads
     // to duplicate results)

cpp/ql/src/Security/CWE/CWE-843/TypeConfusion.ql

@@ -14,48 +14,6 @@ import cpp
 import semmle.code.cpp.dataflow.new.DataFlow
 import Flow::PathGraph
 
-/**
- * Holds if `f` is a field located at byte offset `offset` in `c`.
- *
- * Note that predicate is recursive, so that given the following:
- * ```cpp
- * struct S1 {
- *   int a;
- *   void* b;
- * };
- *
- * struct S2 {
- *   S1 s1;
- *   char c;
- * };
- * ```
- * both `hasAFieldWithOffset(S2, s1, 0)` and `hasAFieldWithOffset(S2, a, 0)`
- * holds.
- */
-predicate hasAFieldWithOffset(Class c, Field f, int offset) {
-  // Base case: `f` is a field in `c`.
-  f = c.getAField() and
-  offset = f.getByteOffset() and
-  not f.getUnspecifiedType().(Class).hasDefinition()
-  or
-  // Otherwise, we find the struct that is a field of `c` which then has
-  // the field `f` as a member.
-  exists(Field g |
-    g = c.getAField() and
-    // Find the field with the largest offset that's less than or equal to
-    // offset. That's the struct we need to search recursively.
-    g =
-      max(Field cand, int candOffset |
-        cand = c.getAField() and
-        candOffset = cand.getByteOffset() and
-        offset >= candOffset
-      |
-        cand order by candOffset
-      ) and
-    hasAFieldWithOffset(g.getUnspecifiedType(), f, offset - g.getByteOffset())
-  )
-}
-
 /** Holds if `f` is the last field of its declaring class. */
 predicate lastField(Field f) {
   exists(Class c | c = f.getDeclaringType() |
@@ -75,7 +33,7 @@ predicate lastField(Field f) {
 bindingset[f1, offset, c2]
 pragma[inline_late]
 predicate hasCompatibleFieldAtOffset(Field f1, int offset, Class c2) {
-  exists(Field f2 | hasAFieldWithOffset(c2, f2, offset) |
+  exists(Field f2 | offset = f2.getOffsetInClass(c2) |
     // Let's not deal with bit-fields for now.
     f2 instanceof BitField
     or
@@ -100,7 +58,7 @@ predicate prefix(Class c1, Class c2) {
     exists(Field f1, int offset |
       // Let's not deal with bit-fields for now.
       not f1 instanceof BitField and
-      hasAFieldWithOffset(c1, f1, offset)
+      offset = f1.getOffsetInClass(c1)
     |
       hasCompatibleFieldAtOffset(f1, offset, c2)
     )
@@ -108,7 +66,7 @@ predicate prefix(Class c1, Class c2) {
     forall(Field f1, int offset |
       // Let's not deal with bit-fields for now.
       not f1 instanceof BitField and
-      hasAFieldWithOffset(c1, f1, offset)
+      offset = f1.getOffsetInClass(c1)
     |
       hasCompatibleFieldAtOffset(f1, offset, c2)
     )

cpp/ql/src/change-notes/2025-01-28-overflow-buffer.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* The "Call to memory access function may overflow buffer" query (`cpp/overflow-buffer`) now produces fewer FPs involving non-static member variables.