Merge branch 'main' into snake_case_pr

Author: erik-krogh

.devcontainer/devcontainer.json

@@ -4,6 +4,6 @@
     "slevesque.vscode-zipexplorer"
   ],
   "settings": {
-    "codeQL.experimentalBqrsParsing": true
+    "codeQL.runningQueries.memory": 2048
   }
 }

change-notes/1.26/analysis-cpp.md

@@ -13,6 +13,7 @@ The following changes in version 1.26 affect C/C++ analysis in all applications.
 
 | **Query**                  | **Expected impact**    | **Change**                                                       |
 |----------------------------|------------------------|------------------------------------------------------------------|
+| Declaration hides parameter (`cpp/declaration-hides-parameter`) | Fewer false positive results | False positives involving template functions have been fixed. |
 | Inconsistent direction of for loop (`cpp/inconsistent-loop-direction`) | Fewer false positive results | The query now accounts for intentional wrapping of an unsigned loop counter. |
 | Overflow in uncontrolled allocation size (`cpp/uncontrolled-allocation-size`) | | The precision of this query has been decreased from "high" to "medium". As a result, the query is still run but results are no longer displayed on LGTM by default. |
 | Comparison result is always the same (`cpp/constant-comparison`) | More correct results | Bounds on expressions involving multiplication can now be determined in more cases. |

change-notes/1.26/analysis-javascript.md

@@ -33,3 +33,4 @@
 
 
 ## Changes to libraries
+* The predicate `TypeAnnotation.hasQualifiedName` now works in more cases when the imported library was not present during extraction.

cpp/ql/src/Best Practices/Hiding/DeclarationHidesParameter.ql

@@ -11,20 +11,35 @@
 
 import cpp
 
+/**
+ * Gets the template that a function `f` is constructed from, or just `f` if it
+ * is not from a template instantiation.
+ */
+Function getConstructedFrom(Function f) {
+  f.isConstructedFrom(result)
+  or
+  not f.isConstructedFrom(_) and
+  result = f
+}
+
 /**
  * Gets the parameter of `f` with name `name`, which has to come from the
  * _definition_ of `f` and not a prototype declaration.
  * We also exclude names from functions that have multiple definitions.
  * This should not happen in a single application but since we
  * have a system wide view it is likely to happen for instance for
  * the main function.
+ *
+ * Note: we use `getConstructedFrom` to ensure that we look at template
+ * functions rather than their instantiations. We get better results this way
+ * as the instantiation is artificial and may have inherited parameter names
+ * from the declaration rather than the definition.
  */
 ParameterDeclarationEntry functionParameterNames(Function f, string name) {
   exists(FunctionDeclarationEntry fe |
     result.getFunctionDeclarationEntry() = fe and
-    fe.getFunction() = f and
+    getConstructedFrom(f).getDefinition() = fe and
     fe.getLocation() = f.getDefinitionLocation() and
-    result.getFile() = fe.getFile() and // Work around CPP-331
     strictcount(f.getDefinitionLocation()) = 1 and
     result.getName() = name
   )

cpp/ql/src/semmle/code/cpp/models/implementations/StdString.qll

@@ -1,6 +1,6 @@
 /**
  * Provides implementation classes modeling `std::string` and other
- * instantiations of`std::basic_string`. See `semmle.code.cpp.models.Models`
+ * instantiations of `std::basic_string`. See `semmle.code.cpp.models.Models`
  * for usage information.
  */
 
@@ -82,6 +82,32 @@ class StdStringData extends TaintFunction {
   }
 }
 
+/**
+ * The `std::string` function `push_back`.
+ */
+class StdStringPush extends TaintFunction {
+  StdStringPush() { this.hasQualifiedName("std", "basic_string", "push_back") }
+
+  override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
+    // flow from parameter to qualifier
+    input.isParameterDeref(0) and
+    output.isQualifierObject()
+  }
+}
+
+/**
+ * The `std::string` functions `front` and `back`.
+ */
+class StdStringFrontBack extends TaintFunction {
+  StdStringFrontBack() { this.hasQualifiedName("std", "basic_string", ["front", "back"]) }
+
+  override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
+    // flow from object to returned reference
+    input.isQualifierObject() and
+    output.isReturnValueDeref()
+  }
+}
+
 /**
  * The `std::string` function `operator+`.
  */
@@ -138,6 +164,11 @@ class StdStringAppend extends TaintFunction {
       output.isQualifierObject() or
       output.isReturnValueDeref()
     )
+    or
+    // reverse flow from returned reference to the qualifier (for writes to
+    // the result)
+    input.isReturnValueDeref() and
+    output.isQualifierObject()
   }
 }
 
@@ -173,6 +204,11 @@ class StdStringAssign extends TaintFunction {
       output.isQualifierObject() or
       output.isReturnValueDeref()
     )
+    or
+    // reverse flow from returned reference to the qualifier (for writes to
+    // the result)
+    input.isReturnValueDeref() and
+    output.isQualifierObject()
   }
 }
 

cpp/ql/src/semmle/code/cpp/rangeanalysis/SimpleRangeAnalysis.qll

@@ -1144,6 +1144,17 @@ private float getPhiLowerBounds(StackVariable v, RangeSsaDefinition phi) {
     if guardLB > defLB then result = guardLB else result = defLB
   )
   or
+  exists(VariableAccess access, float neConstant, float lower |
+    isNEPhi(v, phi, access, neConstant) and
+    lower = getFullyConvertedLowerBounds(access) and
+    if lower = neConstant then result = lower + 1 else result = lower
+  )
+  or
+  exists(VariableAccess access |
+    isUnsupportedGuardPhi(v, phi, access) and
+    result = getFullyConvertedLowerBounds(access)
+  )
+  or
   result = getDefLowerBounds(phi.getAPhiInput(v), v)
 }
 
@@ -1161,6 +1172,17 @@ private float getPhiUpperBounds(StackVariable v, RangeSsaDefinition phi) {
     if guardUB < defUB then result = guardUB else result = defUB
   )
   or
+  exists(VariableAccess access, float neConstant, float upper |
+    isNEPhi(v, phi, access, neConstant) and
+    upper = getFullyConvertedUpperBounds(access) and
+    if upper = neConstant then result = upper - 1 else result = upper
+  )
+  or
+  exists(VariableAccess access |
+    isUnsupportedGuardPhi(v, phi, access) and
+    result = getFullyConvertedUpperBounds(access)
+  )
+  or
   result = getDefUpperBounds(phi.getAPhiInput(v), v)
 }
 
@@ -1423,22 +1445,13 @@ private predicate linearBoundFromGuard(
   //   1. x <= upperbound(RHS)
   //   2. x >= lowerbound(RHS)
   //
-  // For x != RHS, we create trivial bounds:
-  //
-  //   1. x <= typeUpperBound(RHS.getUnspecifiedType())
-  //   2. x >= typeLowerBound(RHS.getUnspecifiedType())
-  //
-  exists(Expr lhs, Expr rhs, boolean isEQ |
+  exists(Expr lhs, Expr rhs |
     linearAccess(lhs, v, p, q) and
-    eqOpWithSwapAndNegate(guard, lhs, rhs, isEQ, branch) and
+    eqOpWithSwapAndNegate(guard, lhs, rhs, true, branch) and
+    getBounds(rhs, boundValue, isLowerBound) and
     strictness = Nonstrict()
-  |
-    // True branch
-    isEQ = true and getBounds(rhs, boundValue, isLowerBound)
-    or
-    // False branch: set the bounds to the min/max for the type.
-    isEQ = false and exprTypeBounds(rhs, boundValue, isLowerBound)
   )
+  // x != RHS and !x are handled elsewhere
 }
 
 /** Utility for `linearBoundFromGuard`. */
@@ -1455,6 +1468,42 @@ private predicate exprTypeBounds(Expr expr, float boundValue, boolean isLowerBou
   isLowerBound = false and boundValue = exprMaxVal(expr.getFullyConverted())
 }
 
+/**
+ * Holds if `(v, phi)` ensures that `access` is not equal to `neConstant`. For
+ * example, the condition `if (x + 1 != 3)` ensures that `x` is not equal to 2.
+ * Only integral types are supported.
+ */
+private predicate isNEPhi(
+  Variable v, RangeSsaDefinition phi, VariableAccess access, float neConstant
+) {
+  exists(
+    ComparisonOperation cmp, boolean branch, Expr linearExpr, Expr rExpr, float p, float q, float r
+  |
+    access.getTarget() = v and
+    phi.isGuardPhi(access, cmp, branch) and
+    eqOpWithSwapAndNegate(cmp, linearExpr, rExpr, false, branch) and
+    v.getUnspecifiedType() instanceof IntegralOrEnumType and // Float `!=` is too imprecise
+    r = getValue(rExpr).toFloat() and
+    linearAccess(linearExpr, access, p, q) and
+    neConstant = (r - q) / p
+  )
+}
+
+/**
+ * Holds if `(v, phi)` constrains the value of `access` but in a way that
+ * doesn't allow this library to constrain the upper or lower bounds of
+ * `access`. An example is `if (x != y)` if neither `x` nor `y` is a
+ * compile-time constant.
+ */
+private predicate isUnsupportedGuardPhi(Variable v, RangeSsaDefinition phi, VariableAccess access) {
+  exists(ComparisonOperation cmp, boolean branch |
+    access.getTarget() = v and
+    phi.isGuardPhi(access, cmp, branch) and
+    eqOpWithSwapAndNegate(cmp, _, _, false, branch) and
+    not isNEPhi(v, phi, access, _)
+  )
+}
+
 cached
 private module SimpleRangeAnalysisCached {
   /**