Merge branch 'main' into no-dtt-in-unbounded-write

Author: MathiasVP

cpp/ql/lib/change-notes/2023-11-08-strsafe-models.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Added models for the `sprintf` variants from the `StrSafe.h` header.

cpp/ql/lib/semmle/code/cpp/models/implementations/Printf.qll

@@ -147,19 +147,32 @@ private class SnprintfImpl extends Snprintf {
 
 /**
  * The Microsoft `StringCchPrintf` function and variants.
+ * See: https://learn.microsoft.com/en-us/windows/win32/api/strsafe/
+ *      and
+ *      https://learn.microsoft.com/en-us/previous-versions/windows/embedded/ms860435(v=msdn.10)
  */
 private class StringCchPrintf extends FormattingFunction {
   StringCchPrintf() {
     this instanceof TopLevelFunction and
-    this.hasGlobalName([
-        "StringCchPrintf", "StringCchPrintfEx", "StringCchPrintf_l", "StringCchPrintf_lEx",
-        "StringCbPrintf", "StringCbPrintfEx", "StringCbPrintf_l", "StringCbPrintf_lEx"
-      ]) and
+    exists(string baseName |
+      baseName in [
+          "StringCchPrintf", //StringCchPrintf(pszDest, cchDest, pszFormat, ...)
+          "StringCchPrintfEx", //StringCchPrintfEx(pszDest,cchDest, ppszDestEnd, pcchRemaining, dwFlags, pszFormat, ...)
+          "StringCchPrintf_l", //StringCchPrintf_l(pszDest, cbDest, pszFormat, locale, ...)
+          "StringCchPrintf_lEx", //StringCchPrintf_lEx(pszDest, cchDest, ppszDestEnd, pcchRemaining, dwFlags, pszFormat, locale, ...)
+          "StringCbPrintf", //StringCbPrintf(pszDest, cbDest, pszFormat, ...)
+          "StringCbPrintfEx", //StringCbPrintfEx(pszDest, cbDest, ppszDestEnd, pcbRemaining, dwFlags, pszFormat, ...)
+          "StringCbPrintf_l", //StringCbPrintf_l(pszDest, cbDest, pszFormat, locale, ...)
+          "StringCbPrintf_lEx" //StringCbPrintf_lEx(pszDest, cbDest, ppszDestEnd, pcbRemaining, dwFlags, pszFormat, locale, ...)
+        ]
+    |
+      this.hasGlobalName(baseName + ["", "A", "W"])
+    ) and
     not exists(this.getDefinition().getFile().getRelativePath())
   }
 
   override int getFormatParameterIndex() {
-    if this.getName().matches("%Ex") then result = 5 else result = 2
+    if this.getName().matches("%Ex" + ["", "A", "W"]) then result = 5 else result = 2
   }
 
   override int getOutputParameterIndex(boolean isStream) { result = 0 and isStream = false }

cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/SemanticExprSpecific.qll

@@ -130,7 +130,7 @@ module SemanticExprConfig {
 
   newtype TSsaVariable =
     TSsaInstruction(IR::Instruction instr) { instr.hasMemoryResult() } or
-    TSsaOperand(IR::Operand op) { op.isDefinitionInexact() }
+    TSsaOperand(IR::PhiInputOperand op) { op.isDefinitionInexact() }
 
   class SsaVariable extends TSsaVariable {
     string toString() { none() }
@@ -139,7 +139,7 @@ module SemanticExprConfig {
 
     IR::Instruction asInstruction() { none() }
 
-    IR::Operand asOperand() { none() }
+    IR::PhiInputOperand asOperand() { none() }
   }
 
   class SsaInstructionVariable extends SsaVariable, TSsaInstruction {
@@ -155,15 +155,15 @@ module SemanticExprConfig {
   }
 
   class SsaOperand extends SsaVariable, TSsaOperand {
-    IR::Operand op;
+    IR::PhiInputOperand op;
 
     SsaOperand() { this = TSsaOperand(op) }
 
     final override string toString() { result = op.toString() }
 
     final override Location getLocation() { result = op.getLocation() }
 
-    final override IR::Operand asOperand() { result = op }
+    final override IR::PhiInputOperand asOperand() { result = op }
   }
 
   predicate explicitUpdate(SsaVariable v, Expr sourceExpr) {
@@ -190,83 +190,25 @@ module SemanticExprConfig {
 
   SemType getSsaVariableType(SsaVariable v) {
     result = getSemanticType(v.asInstruction().getResultIRType())
+    or
+    result = getSemanticType(v.asOperand().getUse().getResultIRType())
   }
 
   BasicBlock getSsaVariableBasicBlock(SsaVariable v) {
     result = v.asInstruction().getBlock()
     or
-    result = v.asOperand().getUse().getBlock()
-  }
-
-  private newtype TReadPosition =
-    TReadPositionBlock(IR::IRBlock block) or
-    TReadPositionPhiInputEdge(IR::IRBlock pred, IR::IRBlock succ) {
-      exists(IR::PhiInputOperand input |
-        pred = input.getPredecessorBlock() and
-        succ = input.getUse().getBlock()
-      )
-    }
-
-  class SsaReadPosition extends TReadPosition {
-    string toString() { none() }
-
-    Location getLocation() { none() }
-
-    predicate hasRead(SsaVariable v) { none() }
-  }
-
-  private class SsaReadPositionBlock extends SsaReadPosition, TReadPositionBlock {
-    IR::IRBlock block;
-
-    SsaReadPositionBlock() { this = TReadPositionBlock(block) }
-
-    final override string toString() { result = block.toString() }
-
-    final override Location getLocation() { result = block.getLocation() }
-
-    final override predicate hasRead(SsaVariable v) {
-      exists(IR::Operand operand | operand.getDef() = v.asInstruction() |
-        not operand instanceof IR::PhiInputOperand and
-        operand.getUse().getBlock() = block
-      )
-    }
-  }
-
-  private class SsaReadPositionPhiInputEdge extends SsaReadPosition, TReadPositionPhiInputEdge {
-    IR::IRBlock pred;
-    IR::IRBlock succ;
-
-    SsaReadPositionPhiInputEdge() { this = TReadPositionPhiInputEdge(pred, succ) }
-
-    final override string toString() { result = pred.toString() + "->" + succ.toString() }
-
-    final override Location getLocation() { result = succ.getLocation() }
-
-    final override predicate hasRead(SsaVariable v) {
-      exists(IR::PhiInputOperand operand | operand.getDef() = v.asInstruction() |
-        operand.getPredecessorBlock() = pred and
-        operand.getUse().getBlock() = succ
-      )
-    }
-  }
-
-  predicate hasReadOfSsaVariable(SsaReadPosition pos, SsaVariable v) { pos.hasRead(v) }
-
-  predicate readBlock(SsaReadPosition pos, BasicBlock block) { pos = TReadPositionBlock(block) }
-
-  predicate phiInputEdge(SsaReadPosition pos, BasicBlock origBlock, BasicBlock phiBlock) {
-    pos = TReadPositionPhiInputEdge(origBlock, phiBlock)
+    result = v.asOperand().getAnyDef().getBlock()
   }
 
-  predicate phiInput(SsaReadPosition pos, SsaVariable phi, SsaVariable input) {
+  /** Holds if `inp` is an input to the phi node along the edge originating in `bb`. */
+  predicate phiInputFromBlock(SsaVariable phi, SsaVariable inp, BasicBlock bb) {
     exists(IR::PhiInputOperand operand |
-      pos = TReadPositionPhiInputEdge(operand.getPredecessorBlock(), operand.getUse().getBlock())
-    |
+      bb = operand.getPredecessorBlock() and
       phi.asInstruction() = operand.getUse() and
       (
-        input.asInstruction() = operand.getDef()
+        inp.asInstruction() = operand.getDef()
         or
-        input.asOperand() = operand
+        inp.asOperand() = operand
       )
     )
   }

cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/SemanticSSA.qll

@@ -31,35 +31,8 @@ class SemSsaPhiNode extends SemSsaVariable {
   SemSsaPhiNode() { Specific::phi(this) }
 
   final SemSsaVariable getAPhiInput() { result = Specific::getAPhiInput(this) }
-}
-
-class SemSsaReadPosition instanceof Specific::SsaReadPosition {
-  final string toString() { result = super.toString() }
-
-  final Specific::Location getLocation() { result = super.getLocation() }
-
-  final predicate hasReadOfVar(SemSsaVariable var) { Specific::hasReadOfSsaVariable(this, var) }
-}
-
-class SemSsaReadPositionPhiInputEdge extends SemSsaReadPosition {
-  SemBasicBlock origBlock;
-  SemBasicBlock phiBlock;
-
-  SemSsaReadPositionPhiInputEdge() { Specific::phiInputEdge(this, origBlock, phiBlock) }
-
-  predicate phiInput(SemSsaPhiNode phi, SemSsaVariable inp) { Specific::phiInput(this, phi, inp) }
-
-  SemBasicBlock getOrigBlock() { result = origBlock }
-
-  SemBasicBlock getPhiBlock() { result = phiBlock }
-}
-
-class SemSsaReadPositionBlock extends SemSsaReadPosition {
-  SemBasicBlock block;
-
-  SemSsaReadPositionBlock() { Specific::readBlock(this, block) }
-
-  SemBasicBlock getBlock() { result = block }
 
-  SemExpr getAnExpr() { result = this.getBlock().getAnExpr() }
+  final predicate hasInputFromBlock(SemSsaVariable inp, SemBasicBlock bb) {
+    Specific::phiInputFromBlock(this, inp, bb)
+  }
 }

cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/analysis/RangeAnalysisImpl.qll

@@ -74,6 +74,8 @@ module Sem implements Semantic {
 
   BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.getASuccessor() }
 
+  int getBlockId1(BasicBlock bb) { result = bb.getUniqueId() }
+
   class Guard = SemGuard;
 
   predicate implies_v2 = semImplies_v2/4;
@@ -92,12 +94,6 @@ module Sem implements Semantic {
 
   class SsaExplicitUpdate = SemSsaExplicitUpdate;
 
-  class SsaReadPosition = SemSsaReadPosition;
-
-  class SsaReadPositionPhiInputEdge = SemSsaReadPositionPhiInputEdge;
-
-  class SsaReadPositionBlock = SemSsaReadPositionBlock;
-
   predicate conversionCannotOverflow(Type fromType, Type toType) {
     SemanticType::conversionCannotOverflow(fromType, toType)
   }

cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/analysis/RangeUtils.qll

@@ -133,33 +133,4 @@ module RangeUtil<DeltaSig D, LangSig<Sem, D> Lang> implements UtilSig<Sem, D> {
     or
     not exists(Lang::getAlternateTypeForSsaVariable(var)) and result = var.getType()
   }
-
-  import Ranking
-}
-
-import Ranking
-
-module Ranking {
-  /**
-   * Holds if `rix` is the number of input edges to `phi`.
-   */
-  predicate maxPhiInputRank(SemSsaPhiNode phi, int rix) {
-    rix = max(int r | rankedPhiInput(phi, _, _, r))
-  }
-
-  /**
-   * Holds if `inp` is an input to `phi` along `edge` and this input has index `r`
-   * in an arbitrary 1-based numbering of the input edges to `phi`.
-   */
-  predicate rankedPhiInput(
-    SemSsaPhiNode phi, SemSsaVariable inp, SemSsaReadPositionPhiInputEdge edge, int r
-  ) {
-    edge.phiInput(phi, inp) and
-    edge =
-      rank[r](SemSsaReadPositionPhiInputEdge e |
-        e.phiInput(phi, _)
-      |
-        e order by e.getOrigBlock().getUniqueId()
-      )
-  }
 }