Ruby: update crate versions

JS: DB reads as taint sources

.codeqlmanifest.json

@@ -4,10 +4,12 @@
         "*/ql/lib/qlpack.yml",
         "*/ql/test/qlpack.yml",
         "*/ql/examples/qlpack.yml",
-        "*/upgrades/qlpack.yml",
         "cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/tainted/qlpack.yml",
         "javascript/ql/experimental/adaptivethreatmodeling/lib/qlpack.yml",
         "javascript/ql/experimental/adaptivethreatmodeling/src/qlpack.yml",
+        "csharp/ql/campaigns/Solorigate/lib/qlpack.yml",
+        "csharp/ql/campaigns/Solorigate/src/qlpack.yml",
+        "csharp/ql/campaigns/Solorigate/test/qlpack.yml",
         "misc/legacy-support/*/qlpack.yml",
         "misc/suite-helpers/qlpack.yml",
         "ruby/extractor-pack/codeql-extractor.yml",

README.md

@@ -1,11 +1,11 @@
 # CodeQL
 
-This open source repository contains the standard CodeQL libraries and queries that power [LGTM](https://lgtm.com) and the other CodeQL products that [GitHub](https://github.com) makes available to its customers worldwide. For the queries, libraries, and extractor that power Go analysis, visit the [CodeQL for Go repository](https://github.com/github/codeql-go).
+This open source repository contains the standard CodeQL libraries and queries that power [GitHub Advanced Security](https://github.com/features/security/code) and the other application security products that [GitHub](https://github.com/features/security/) makes available to its customers worldwide. For the queries, libraries, and extractor that power Go analysis, visit the [CodeQL for Go repository](https://github.com/github/codeql-go).
 
 ## How do I learn CodeQL and run queries?
 
 There is [extensive documentation](https://codeql.github.com/docs/) on getting started with writing CodeQL.
-You can use the [interactive query console](https://lgtm.com/help/lgtm/using-query-console) on LGTM.com or the [CodeQL for Visual Studio Code](https://codeql.github.com/docs/codeql-for-visual-studio-code/) extension to try out your queries on any open source project that's currently being analyzed.
+You can use the [CodeQL for Visual Studio Code](https://codeql.github.com/docs/codeql-for-visual-studio-code/) extension or the [interactive query console](https://lgtm.com/help/lgtm/using-query-console) on LGTM.com (Semmle Legacy product) to try out your queries on any open source project that's currently being analyzed.
 
 ## Contributing
 
@@ -13,7 +13,7 @@ We welcome contributions to our standard library and standard checks. Do you hav
 
 ## License
 
-The code in this repository is licensed under the [MIT License](LICENSE) by [GitHub](https://github.com).
+The code in this repository is licensed under the [MIT License](LICENSE) by [GitHub](https://github.com). The use of CodeQL on open source code is licensed under specific [Terms & Conditions](https://securitylab.github.com/tools/codeql/license/) UNLESS you have a commercial license in place. If you'd like to use CodeQL with a commercial codebase, please [contact us](https://github.com/enterprise/contact) for further help.
 
 ## Visual Studio Code integration
 

cpp/ql/lib/qlpack.yml

@@ -4,5 +4,4 @@ groups: cpp
 dbscheme: semmlecode.cpp.dbscheme
 extractor: cpp
 library: true
-dependencies:
+upgrades: upgrades
-  codeql/cpp-upgrades: ^0.0.3

cpp/ql/lib/semmle/code/cpp/dataflow/internal/FlowVar.qll

@@ -435,7 +435,7 @@ module FlowVar_internal {
       parameterIsNonConstReference(p) and
       p = v and
       // This definition reaches the exit node of the function CFG
-      getAReachedBlockVarSBB(this).getANode() = p.getFunction()
+      getAReachedBlockVarSBB(this).getEnd() = p.getFunction()
     }
 
     override predicate definedByInitialValue(StackVariable lsv) {

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

@@ -63,7 +63,7 @@ private module VirtualDispatch {
         this.flowsFrom(other, allowOtherFromArg)
       |
         // Call argument
-        exists(DataFlowCall call, int i |
+        exists(DataFlowCall call, Position i |
           other
               .(DataFlow::ParameterNode)
               .isParameterOf(pragma[only_bind_into](call).getStaticCallTarget(), i) and
@@ -268,16 +268,6 @@ Function viableImplInCallContext(CallInstruction call, CallInstruction ctx) {
   )
 }
 
-/** A parameter position represented by an integer. */
-class ParameterPosition extends int {
-  ParameterPosition() { any(ParameterNode p).isParameterOf(_, this) }
-}
-
-/** An argument position represented by an integer. */
-class ArgumentPosition extends int {
-  ArgumentPosition() { any(ArgumentNode a).argumentOf(_, this) }
-}
-
 /** Holds if arguments at position `apos` match parameters at position `ppos`. */
 pragma[inline]
 predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) { ppos = apos }

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

@@ -27,7 +27,7 @@ abstract class ArgumentNode extends OperandNode {
    * Holds if this argument occurs at the given position in the given call.
    * The instance argument is considered to have index `-1`.
    */
-  abstract predicate argumentOf(DataFlowCall call, int pos);
+  abstract predicate argumentOf(DataFlowCall call, ArgumentPosition pos);
 
   /** Gets the call in which this node is an argument. */
   DataFlowCall getCall() { this.argumentOf(result, _) }
@@ -42,7 +42,9 @@ private class PrimaryArgumentNode extends ArgumentNode {
 
   PrimaryArgumentNode() { exists(CallInstruction call | op = call.getAnArgumentOperand()) }
 
-  override predicate argumentOf(DataFlowCall call, int pos) { op = call.getArgumentOperand(pos) }
+  override predicate argumentOf(DataFlowCall call, ArgumentPosition pos) {
+    op = call.getArgumentOperand(pos.(DirectPosition).getIndex())
+  }
 
   override string toString() {
     exists(Expr unconverted |
@@ -71,9 +73,9 @@ private class SideEffectArgumentNode extends ArgumentNode {
 
   SideEffectArgumentNode() { op = read.getSideEffectOperand() }
 
-  override predicate argumentOf(DataFlowCall call, int pos) {
+  override predicate argumentOf(DataFlowCall call, ArgumentPosition pos) {
     read.getPrimaryInstruction() = call and
-    pos = getArgumentPosOfSideEffect(read.getIndex())
+    pos.(IndirectionPosition).getIndex() = read.getIndex()
   }
 
   override string toString() {
@@ -90,6 +92,54 @@ private class SideEffectArgumentNode extends ArgumentNode {
   }
 }
 
+/** A parameter position represented by an integer. */
+class ParameterPosition = Position;
+
+/** An argument position represented by an integer. */
+class ArgumentPosition = Position;
+
+class Position extends TPosition {
+  abstract string toString();
+}
+
+class DirectPosition extends TDirectPosition {
+  int index;
+
+  DirectPosition() { this = TDirectPosition(index) }
+
+  string toString() {
+    index = -1 and
+    result = "this"
+    or
+    index != -1 and
+    result = index.toString()
+  }
+
+  int getIndex() { result = index }
+}
+
+class IndirectionPosition extends TIndirectionPosition {
+  int index;
+
+  IndirectionPosition() { this = TIndirectionPosition(index) }
+
+  string toString() {
+    index = -1 and
+    result = "this"
+    or
+    index != -1 and
+    result = index.toString()
+  }
+
+  int getIndex() { result = index }
+}
+
+newtype TPosition =
+  TDirectPosition(int index) { exists(any(CallInstruction c).getArgument(index)) } or
+  TIndirectionPosition(int index) {
+    exists(ReadSideEffectInstruction instr | instr.getIndex() = index)
+  }
+
 private newtype TReturnKind =
   TNormalReturnKind() or
   TIndirectReturnKind(ParameterIndex index)

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

@@ -490,19 +490,6 @@ class ExprNode extends InstructionNode {
   override string toString() { result = this.asConvertedExpr().toString() }
 }
 
-/**
- * INTERNAL: do not use. Translates a parameter/argument index into a negative
- * number that denotes the index of its side effect (pointer indirection).
- */
-bindingset[index]
-int getArgumentPosOfSideEffect(int index) {
-  // -1 -> -2
-  //  0 -> -3
-  //  1 -> -4
-  // ...
-  result = -3 - index
-}
-
 /**
  * The value of a parameter at function entry, viewed as a node in a data
  * flow graph. This includes both explicit parameters such as `x` in `f(x)`
@@ -525,7 +512,7 @@ class ParameterNode extends InstructionNode {
    * implicit `this` parameter is considered to have position `-1`, and
    * pointer-indirection parameters are at further negative positions.
    */
-  predicate isParameterOf(Function f, int pos) { none() } // overridden by subclasses
+  predicate isParameterOf(Function f, ParameterPosition pos) { none() } // overridden by subclasses
 }
 
 /** An explicit positional parameter, not including `this` or `...`. */
@@ -534,8 +521,8 @@ private class ExplicitParameterNode extends ParameterNode {
 
   ExplicitParameterNode() { exists(instr.getParameter()) }
 
-  override predicate isParameterOf(Function f, int pos) {
+  override predicate isParameterOf(Function f, ParameterPosition pos) {
-    f.getParameter(pos) = instr.getParameter()
+    f.getParameter(pos.(DirectPosition).getIndex()) = instr.getParameter()
   }
 
   /** Gets the `Parameter` associated with this node. */
@@ -550,8 +537,8 @@ class ThisParameterNode extends ParameterNode {
 
   ThisParameterNode() { instr.getIRVariable() instanceof IRThisVariable }
 
-  override predicate isParameterOf(Function f, int pos) {
+  override predicate isParameterOf(Function f, ParameterPosition pos) {
-    pos = -1 and instr.getEnclosingFunction() = f
+    pos.(DirectPosition).getIndex() = -1 and instr.getEnclosingFunction() = f
   }
 
   override string toString() { result = "this" }
@@ -561,12 +548,12 @@ class ThisParameterNode extends ParameterNode {
 class ParameterIndirectionNode extends ParameterNode {
   override InitializeIndirectionInstruction instr;
 
-  override predicate isParameterOf(Function f, int pos) {
+  override predicate isParameterOf(Function f, ParameterPosition pos) {
     exists(int index |
       instr.getEnclosingFunction() = f and
       instr.hasIndex(index)
     |
-      pos = getArgumentPosOfSideEffect(index)
+      pos.(IndirectionPosition).getIndex() = index
     )
   }
 

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

@@ -659,4 +659,15 @@ module Consistency {
     not phiHasInputFromBlock(_, def, _) and
     not uncertainWriteDefinitionInput(_, def)
   }
+
+  query predicate notDominatedByDef(RelevantDefinition def, SourceVariable v, BasicBlock bb, int i) {
+    exists(BasicBlock bbDef, int iDef | def.definesAt(v, bbDef, iDef) |
+      ssaDefReachesReadWithinBlock(v, def, bb, i) and
+      (bb != bbDef or i < iDef)
+      or
+      ssaDefReachesRead(v, def, bb, i) and
+      not ssaDefReachesReadWithinBlock(v, def, bb, i) and
+      not def.definesAt(v, getImmediateBasicBlockDominator*(bb), _)
+    )
+  }
 }

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

@@ -51,16 +51,6 @@ private newtype TDefOrUse =
   TExplicitUse(Operand op) { isExplicitUse(op) } or
   TReturnParamIndirection(Operand op) { returnParameterIndirection(op, _) }
 
-pragma[nomagic]
-private int getRank(DefOrUse defOrUse, IRBlock block) {
-  defOrUse =
-    rank[result](int i, DefOrUse cand |
-      block.getInstruction(i) = toInstruction(cand)
-    |
-      cand order by i
-    )
-}
-
 private class DefOrUse extends TDefOrUse {
   /** Gets the instruction associated with this definition, if any. */
   Instruction asDef() { none() }
@@ -74,9 +64,10 @@ private class DefOrUse extends TDefOrUse {
   /** Gets the block of this definition or use. */
   abstract IRBlock getBlock();
 
-  /** Holds if this definition or use has rank `rank` in block `block`. */
+  /** Holds if this definition or use has index `index` in block `block`. */
-  cached
+  final predicate hasIndexInBlock(IRBlock block, int index) {
-  final predicate hasRankInBlock(IRBlock block, int rnk) { rnk = getRank(this, block) }
+    block.getInstruction(index) = toInstruction(this)
+  }
 
   /** Gets the location of this element. */
   abstract Cpp::Location getLocation();
@@ -313,8 +304,8 @@ cached
 private module Cached {
   private predicate defUseFlow(Node nodeFrom, Node nodeTo) {
     exists(IRBlock bb1, int i1, IRBlock bb2, int i2, DefOrUse defOrUse, Use use |
-      defOrUse.hasRankInBlock(bb1, i1) and
+      defOrUse.hasIndexInBlock(bb1, i1) and
-      use.hasRankInBlock(bb2, i2) and
+      use.hasIndexInBlock(bb2, i2) and
       adjacentDefRead(_, bb1, i1, bb2, i2) and
       nodeFrom.asInstruction() = toInstruction(defOrUse) and
       flowOutOfAddressStep(use.getOperand(), nodeTo)
@@ -326,9 +317,9 @@ private module Cached {
     exists(IRBlock bb1, int i1, IRBlock bb2, int i2, Def def, Use use |
       nodeFrom.isTerminal() and
       def.getInstruction() = nodeFrom.getStoreInstruction() and
-      def.hasRankInBlock(bb1, i1) and
+      def.hasIndexInBlock(bb1, i1) and
       adjacentDefRead(_, bb1, i1, bb2, i2) and
-      use.hasRankInBlock(bb2, i2) and
+      use.hasIndexInBlock(bb2, i2) and
       flowOutOfAddressStep(use.getOperand(), nodeTo)
     )
     or
@@ -359,8 +350,8 @@ private module Cached {
 
   private predicate fromReadNode(ReadNode nodeFrom, Node nodeTo) {
     exists(IRBlock bb1, int i1, IRBlock bb2, int i2, Use use1, Use use2 |
-      use1.hasRankInBlock(bb1, i1) and
+      use1.hasIndexInBlock(bb1, i1) and
-      use2.hasRankInBlock(bb2, i2) and
+      use2.hasIndexInBlock(bb2, i2) and
       use1.getOperand().getDef() = nodeFrom.getInstruction() and
       adjacentDefRead(_, bb1, i1, bb2, i2) and
       flowOutOfAddressStep(use2.getOperand(), nodeTo)
@@ -371,7 +362,7 @@ private module Cached {
     exists(PhiNode phi, Use use, IRBlock block, int rnk |
       phi = nodeFrom.getPhiNode() and
       adjacentDefRead(phi, _, _, block, rnk) and
-      use.hasRankInBlock(block, rnk) and
+      use.hasIndexInBlock(block, rnk) and
       flowOutOfAddressStep(use.getOperand(), nodeTo)
     )
   }
@@ -379,7 +370,7 @@ private module Cached {
   private predicate toPhiNode(Node nodeFrom, SsaPhiNode nodeTo) {
     // Flow to phi nodes
     exists(Def def, IRBlock block, int rnk |
-      def.hasRankInBlock(block, rnk) and
+      def.hasIndexInBlock(block, rnk) and
       nodeTo.hasInputAtRankInBlock(block, rnk)
     |
       exists(StoreNodeInstr storeNode |
@@ -512,8 +503,8 @@ private module Cached {
     |
       store = def.getInstruction() and
       store.getSourceValueOperand() = operand and
-      def.hasRankInBlock(block1, rnk1) and
+      def.hasIndexInBlock(block1, rnk1) and
-      use.hasRankInBlock(block2, rnk2) and
+      use.hasIndexInBlock(block2, rnk2) and
       adjacentDefRead(_, block1, rnk1, block2, rnk2)
     |
       // The shared SSA library has determined that `use` is the next use of the operand
@@ -543,12 +534,12 @@ private module Cached {
     not operand = getSourceAddressOperand(_) and
     exists(Use use1, Use use2, IRBlock block1, int rnk1, IRBlock block2, int rnk2 |
       use1.getOperand() = operand and
-      use1.hasRankInBlock(block1, rnk1) and
+      use1.hasIndexInBlock(block1, rnk1) and
       // Don't flow to the next use if this use is part of a store operation that totally
       // overrides a variable.
       not explicitWrite(true, _, use1.getOperand().getDef()) and
       adjacentDefRead(_, block1, rnk1, block2, rnk2) and
-      use2.hasRankInBlock(block2, rnk2) and
+      use2.hasIndexInBlock(block2, rnk2) and
       flowOutOfAddressStep(use2.getOperand(), nodeTo)
     )
     or
@@ -620,7 +611,7 @@ import Cached
 predicate variableWrite(IRBlock bb, int i, SourceVariable v, boolean certain) {
   DataFlowImplCommon::forceCachingInSameStage() and
   exists(Def def |
-    def.hasRankInBlock(bb, i) and
+    def.hasIndexInBlock(bb, i) and
     v = def.getSourceVariable() and
     (if def.isCertain() then certain = true else certain = false)
   )
@@ -632,7 +623,7 @@ predicate variableWrite(IRBlock bb, int i, SourceVariable v, boolean certain) {
  */
 predicate variableRead(IRBlock bb, int i, SourceVariable v, boolean certain) {
   exists(Use use |
-    use.hasRankInBlock(bb, i) and
+    use.hasIndexInBlock(bb, i) and
     v = use.getSourceVariable() and
     certain = true
   )

cpp/ql/lib/semmle/code/cpp/security/Overflow.qll

@@ -9,6 +9,7 @@ import semmle.code.cpp.controlflow.Dominance
 private import semmle.code.cpp.valuenumbering.GlobalValueNumbering
 import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
 import semmle.code.cpp.rangeanalysis.RangeAnalysisUtils
+import semmle.code.cpp.controlflow.Guards
 
 /**
  * Holds if the value of `use` is guarded using `abs`.
@@ -16,53 +17,16 @@ import semmle.code.cpp.rangeanalysis.RangeAnalysisUtils
 predicate guardedAbs(Operation e, Expr use) {
   exists(FunctionCall fc | fc.getTarget().getName() = ["abs", "labs", "llabs", "imaxabs"] |
     fc.getArgument(0).getAChild*() = use and
-    guardedLesser(e, fc)
+    exists(GuardCondition c | c.ensuresLt(fc, _, _, e.getBasicBlock(), true))
   )
 }
 
-/**
- * Gets the position of `stmt` in basic block `block` (this is a thin layer
- * over `BasicBlock.getNode`, intended to improve performance).
- */
-pragma[noinline]
-private int getStmtIndexInBlock(BasicBlock block, Stmt stmt) { block.getNode(result) = stmt }
-
-pragma[inline]
-private predicate stmtDominates(Stmt dominator, Stmt dominated) {
-  // In same block
-  exists(BasicBlock block, int dominatorIndex, int dominatedIndex |
-    dominatorIndex = getStmtIndexInBlock(block, dominator) and
-    dominatedIndex = getStmtIndexInBlock(block, dominated) and
-    dominatedIndex >= dominatorIndex
-  )
-  or
-  // In (possibly) different blocks
-  bbStrictlyDominates(dominator.getBasicBlock(), dominated.getBasicBlock())
-}
-
 /**
  * Holds if the value of `use` is guarded to be less than something, and `e`
  * is in code controlled by that guard (where the guard condition held).
  */
-pragma[nomagic]
 predicate guardedLesser(Operation e, Expr use) {
-  exists(IfStmt c, RelationalOperation guard |
+  exists(GuardCondition c | c.ensuresLt(use, _, _, e.getBasicBlock(), true))
-    use = guard.getLesserOperand().getAChild*() and
-    guard = c.getControllingExpr().getAChild*() and
-    stmtDominates(c.getThen(), e.getEnclosingStmt())
-  )
-  or
-  exists(Loop c, RelationalOperation guard |
-    use = guard.getLesserOperand().getAChild*() and
-    guard = c.getControllingExpr().getAChild*() and
-    stmtDominates(c.getStmt(), e.getEnclosingStmt())
-  )
-  or
-  exists(ConditionalExpr c, RelationalOperation guard |
-    use = guard.getLesserOperand().getAChild*() and
-    guard = c.getCondition().getAChild*() and
-    c.getThen().getAChild*() = e
-  )
   or
   guardedAbs(e, use)
 }
@@ -71,25 +35,8 @@ predicate guardedLesser(Operation e, Expr use) {
  * Holds if the value of `use` is guarded to be greater than something, and `e`
  * is in code controlled by that guard (where the guard condition held).
  */
-pragma[nomagic]
 predicate guardedGreater(Operation e, Expr use) {
-  exists(IfStmt c, RelationalOperation guard |
+  exists(GuardCondition c | c.ensuresLt(use, _, _, e.getBasicBlock(), false))
-    use = guard.getGreaterOperand().getAChild*() and
-    guard = c.getControllingExpr().getAChild*() and
-    stmtDominates(c.getThen(), e.getEnclosingStmt())
-  )
-  or
-  exists(Loop c, RelationalOperation guard |
-    use = guard.getGreaterOperand().getAChild*() and
-    guard = c.getControllingExpr().getAChild*() and
-    stmtDominates(c.getStmt(), e.getEnclosingStmt())
-  )
-  or
-  exists(ConditionalExpr c, RelationalOperation guard |
-    use = guard.getGreaterOperand().getAChild*() and
-    guard = c.getCondition().getAChild*() and
-    c.getThen().getAChild*() = e
-  )
   or
   guardedAbs(e, use)
 }

cpp/ql/src/Likely Bugs/AmbiguouslySignedBitField.ql

@@ -26,6 +26,8 @@ where
   // At least for C programs on Windows, BOOL is a common typedef for a type
   // representing BoolType.
   not bf.getType().hasName("BOOL") and
+  // GLib's gboolean is a typedef for a type representing BoolType.
+  not bf.getType().hasName("gboolean") and
   // If this is true, then there cannot be unsigned sign extension or overflow.
   not bf.getDeclaredNumBits() = bf.getType().getSize() * 8 and
   not bf.isAnonymous() and

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

@@ -5,7 +5,7 @@
  * @kind path-problem
  * @problem.severity warning
  * @security-severity 8.6
- * @precision medium
+ * @precision high
  * @id cpp/uncontrolled-arithmetic
  * @tags security
  *       external/cwe/cwe-190
@@ -82,8 +82,11 @@ predicate missingGuard(VariableAccess va, string effect) {
     op.getUnspecifiedType().(IntegralType).isUnsigned() and
     not op instanceof MulExpr
     or
-    // overflow
+    // overflow - only report signed integer overflow since unsigned overflow
-    missingGuardAgainstOverflow(op, va) and effect = "overflow"
+    // is well-defined.
+    op.getUnspecifiedType().(IntegralType).isSigned() and
+    missingGuardAgainstOverflow(op, va) and
+    effect = "overflow"
   )
 }
 

cpp/ql/src/Security/CWE/CWE-367/TOCTOUFilesystemRace.ql

@@ -89,11 +89,13 @@ predicate referenceTo(Expr source, Expr use) {
   )
 }
 
-from Expr check, Expr checkPath, FunctionCall use, Expr usePath
+pragma[noinline]
-where
+predicate statCallWithPointer(Expr checkPath, Expr call, Expr e, Variable v) {
-  // `check` looks like a check on a filename
+  call = stat(checkPath, e) and
-  (
+  e.getAChild*().(VariableAccess).getTarget() = v
-    (
+}
+
+predicate checksPath(Expr check, Expr checkPath) {
   // either:
   // an access check
   check = accessCheck(checkPath)
@@ -105,29 +107,65 @@ where
   // (morally, this should be a use-use pair, but it seems unlikely
   // that this variable will get reused in practice)
   exists(Expr call, Expr e, Variable v |
-        call = stat(checkPath, e) and
+    statCallWithPointer(checkPath, call, e, v) and
-        e.getAChild*().(VariableAccess).getTarget() = v and
     check.(VariableAccess).getTarget() = v and
     not e.getAChild*() = check // the call that writes to the pointer is not where the pointer is checked.
   )
+}
+
+pragma[nomagic]
+predicate checkPathControlsUse(Expr check, Expr checkPath, Expr use) {
+  exists(GuardCondition guard | referenceTo(check, guard.getAChild*()) |
+    guard.controls(use.getBasicBlock(), _)
   ) and
+  checksPath(pragma[only_bind_into](check), checkPath)
+}
+
+pragma[nomagic]
+predicate fileNameOperationControlsUse(Expr check, Expr checkPath, Expr use) {
+  exists(GuardCondition guard | referenceTo(check, guard.getAChild*()) |
+    guard.controls(use.getBasicBlock(), _)
+  ) and
+  pragma[only_bind_into](check) = filenameOperation(checkPath)
+}
+
+predicate checkUse(Expr check, Expr checkPath, FunctionCall use, Expr usePath) {
+  // `check` is part of a guard that controls `use`
+  checkPathControlsUse(check, checkPath, use) and
+  // `check` looks like a check on a filename
+  checksPath(check, checkPath) and
   // `op` looks like an operation on a filename
   use = filenameOperation(usePath)
   or
+  // `check` is part of a guard that controls `use`
+  fileNameOperationControlsUse(check, checkPath, use) and
   // another filename operation (null pointers can indicate errors)
   check = filenameOperation(checkPath) and
   // `op` looks like a sensitive operation on a filename
   use = sensitiveFilenameOperation(usePath)
-  ) and
+}
+
+pragma[noinline]
+predicate isCheckedPath(
+  Expr check, SsaDefinition def, StackVariable v, FunctionCall use, Expr usePath, Expr checkPath
+) {
+  checkUse(check, checkPath, use, usePath) and
+  def.getAUse(v) = checkPath
+}
+
+pragma[noinline]
+predicate isUsedPath(
+  Expr check, SsaDefinition def, StackVariable v, FunctionCall use, Expr usePath, Expr checkPath
+) {
+  checkUse(check, checkPath, use, usePath) and
+  def.getAUse(v) = usePath
+}
+
+from Expr check, Expr checkPath, FunctionCall use, Expr usePath, SsaDefinition def, StackVariable v
+where
   // `checkPath` and `usePath` refer to the same SSA variable
-  exists(SsaDefinition def, StackVariable v |
+  isCheckedPath(check, def, v, use, usePath, checkPath) and
-    def.getAUse(v) = checkPath and def.getAUse(v) = usePath
+  isUsedPath(check, def, v, use, usePath, checkPath)
-  ) and
-  // the return value of `check` is used (possibly with one step of
-  // variable indirection) in a guard which controls `use`
-  exists(GuardCondition guard | referenceTo(check, guard.getAChild*()) |
-    guard.controls(use.(ControlFlowNode).getBasicBlock(), _)
-  )
 select use,
   "The $@ being operated upon was previously $@, but the underlying file may have been changed since then.",
   usePath, "filename", check, "checked"

cpp/ql/src/change-notes/2021-12-30-ambiguously-signed-bit-field.md

@@ -0,0 +1,5 @@
+---
+category: minorAnalysis
+---
+* Added exception for GLib's gboolean to cpp/ambiguously-signed-bit-field.
+  This change reduces the number of false positives in the query.

cpp/ql/src/change-notes/2022-01-05-promote-uncontrolled-arithmetic.md

@@ -0,0 +1,4 @@
+---
+category: newQuery
+---
+* The "Uncontrolled data in arithmetic expression" (cpp/uncontrolled-arithmetic) query has been enhanced to reduce false positive results and its @precision increased to high.

cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/ArithmeticUncontrolled/ArithmeticUncontrolled.expected

@@ -35,8 +35,6 @@ edges
 | test.cpp:190:10:190:13 | call to rand | test.cpp:205:7:205:7 | y |
 | test.cpp:190:10:190:13 | call to rand | test.cpp:208:7:208:7 | y |
 | test.cpp:215:11:215:14 | call to rand | test.cpp:219:8:219:8 | x |
-| test.cpp:223:20:223:23 | call to rand | test.cpp:227:8:227:8 | x |
-| test.cpp:223:20:223:25 | (unsigned int)... | test.cpp:227:8:227:8 | x |
 nodes
 | test.c:18:13:18:16 | call to rand | semmle.label | call to rand |
 | test.c:21:17:21:17 | r | semmle.label | r |
@@ -92,9 +90,6 @@ nodes
 | test.cpp:208:7:208:7 | y | semmle.label | y |
 | test.cpp:215:11:215:14 | call to rand | semmle.label | call to rand |
 | test.cpp:219:8:219:8 | x | semmle.label | x |
-| test.cpp:223:20:223:23 | call to rand | semmle.label | call to rand |
-| test.cpp:223:20:223:25 | (unsigned int)... | semmle.label | (unsigned int)... |
-| test.cpp:227:8:227:8 | x | semmle.label | x |
 subpaths
 #select
 | test.c:21:17:21:17 | r | test.c:18:13:18:16 | call to rand | test.c:21:17:21:17 | r | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.c:18:13:18:16 | call to rand | Uncontrolled value |
@@ -125,5 +120,3 @@ subpaths
 | test.cpp:205:7:205:7 | y | test.cpp:190:10:190:13 | call to rand | test.cpp:205:7:205:7 | y | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.cpp:190:10:190:13 | call to rand | Uncontrolled value |
 | test.cpp:208:7:208:7 | y | test.cpp:190:10:190:13 | call to rand | test.cpp:208:7:208:7 | y | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.cpp:190:10:190:13 | call to rand | Uncontrolled value |
 | test.cpp:219:8:219:8 | x | test.cpp:215:11:215:14 | call to rand | test.cpp:219:8:219:8 | x | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.cpp:215:11:215:14 | call to rand | Uncontrolled value |
-| test.cpp:227:8:227:8 | x | test.cpp:223:20:223:23 | call to rand | test.cpp:227:8:227:8 | x | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.cpp:223:20:223:23 | call to rand | Uncontrolled value |
-| test.cpp:227:8:227:8 | x | test.cpp:223:20:223:25 | (unsigned int)... | test.cpp:227:8:227:8 | x | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.cpp:223:20:223:23 | call to rand | Uncontrolled value |

cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/ArithmeticUncontrolled/test.c

@@ -157,3 +157,11 @@ void moreTests() {
     r = r - 100; // BAD
   }
 }
+
+void guarded_test(unsigned p) {
+  unsigned data = (unsigned int)rand();
+  if (p >= data) {
+    return;
+  }
+  unsigned z = data - p; // GOOD
+}