Merge branch 'main' into python-add-global-flow-steps

Author: tausbn

change-notes/1.26/analysis-cpp.md

@@ -23,7 +23,7 @@ The following changes in version 1.26 affect C/C++ analysis in all applications.
 * The QL class `Block`, denoting the `{ ... }` statement, is renamed to `BlockStmt`.
 * The models library now models many taint flows through `std::array`, `std::vector`, `std::deque`, `std::list` and `std::forward_list`.
 * The models library now models many more taint flows through `std::string`.
-* The models library now models some taint flows through `std::ostream`.
+* The models library now models many taint flows through `std::istream` and `std::ostream`.
 * The models library now models some taint flows through `std::shared_ptr`, `std::unique_ptr`, `std::make_shared` and `std::make_unique`.
 * The `SimpleRangeAnalysis` library now supports multiplications of the form
   `e1 * e2` and `x *= e2` when `e1` and `e2` are unsigned or constant.

change-notes/1.26/analysis-javascript.md

@@ -3,16 +3,21 @@
 ## General improvements
 
 * Support for the following frameworks and libraries has been improved:
+  - [bluebird](https://www.npmjs.com/package/bluebird)
+  - [express](https://www.npmjs.com/package/express)
   - [fast-json-stable-stringify](https://www.npmjs.com/package/fast-json-stable-stringify)
   - [fast-safe-stringify](https://www.npmjs.com/package/fast-safe-stringify)
+  - [http](https://nodejs.org/api/http.html)
   - [javascript-stringify](https://www.npmjs.com/package/javascript-stringify)
   - [js-stringify](https://www.npmjs.com/package/js-stringify)
   - [json-stable-stringify](https://www.npmjs.com/package/json-stable-stringify)
   - [json-stringify-safe](https://www.npmjs.com/package/json-stringify-safe)
   - [json3](https://www.npmjs.com/package/json3)
+  - [lodash](https://www.npmjs.com/package/lodash)
   - [object-inspect](https://www.npmjs.com/package/object-inspect)
   - [pretty-format](https://www.npmjs.com/package/pretty-format)
   - [stringify-object](https://www.npmjs.com/package/stringify-object)
+  - [underscore](https://www.npmjs.com/package/underscore)
 
 * Analyzing files with the ".cjs" extension is now supported.
 
@@ -32,6 +37,7 @@
 | Unused loop iteration variable (`js/unused-loop-variable`) | Fewer results | This query no longer flags variables in a destructuring array assignment that are not the last variable in the destructed array. |
 | Unsafe shell command constructed from library input (`js/shell-command-constructed-from-input`) | More results | This query now recognizes more commands where colon, dash, and underscore are used. |
 | Unsafe jQuery plugin (`js/unsafe-jquery-plugin`) | More results | This query now detects more unsafe uses of nested option properties. |
+| Client-side URL redirect (`js/client-side-unvalidated-url-redirection`) | More results | This query now recognizes some unsafe uses of `importScripts()` inside WebWorkers. |
 
 
 ## Changes to libraries

cpp/ql/src/semmle/code/cpp/Location.qll

@@ -105,16 +105,23 @@ class Location extends @location {
 }
 
 /**
+ * DEPRECATED: Use `Location` instead.
  * A location of an element. Not used for expressions or statements, which
  * instead use LocationExpr and LocationStmt respectively.
  */
-library class LocationDefault extends Location, @location_default { }
+deprecated library class LocationDefault extends Location, @location_default { }
 
-/** A location of a statement. */
-library class LocationStmt extends Location, @location_stmt { }
+/**
+ * DEPRECATED: Use `Location` instead.
+ * A location of a statement.
+ */
+deprecated library class LocationStmt extends Location, @location_stmt { }
 
-/** A location of an expression. */
-library class LocationExpr extends Location, @location_expr { }
+/**
+ * DEPRECATED: Use `Location` instead.
+ * A location of an expression.
+ */
+deprecated library class LocationExpr extends Location, @location_expr { }
 
 /**
  * Gets the length of the longest line in file `f`.

cpp/ql/src/semmle/code/cpp/controlflow/internal/ConstantExprs.qll

@@ -279,20 +279,62 @@ private predicate reachableRecursive(ControlFlowNode n) {
   reachableRecursive(n.getAPredecessor())
 }
 
+/** Holds if `e` is a compile time constant with integer value `val`. */
 private predicate compileTimeConstantInt(Expr e, int val) {
-  val = e.getFullyConverted().getValue().toInt() and
-  not e instanceof StringLiteral and
-  not exists(Expr e1 | e1.getConversion() = e) // only values for fully converted expressions
+  (
+    // If we have an integer value then we are done.
+    if exists(e.getValue().toInt())
+    then val = e.getValue().toInt()
+    else
+      // Otherwise, if we are a conversion of another expression with an
+      // integer value, and that value can be converted into our type,
+      // then we have that value.
+      exists(Expr x, int valx |
+        x.getConversion() = e and
+        compileTimeConstantInt(x, valx) and
+        val = convertIntToType(valx, e.getType().getUnspecifiedType())
+      )
+  ) and
+  // If our unconverted expression is a string literal `"123"`, then we
+  // do not have integer value `123`.
+  not e.getUnconverted() instanceof StringLiteral
 }
 
-library class CompileTimeConstantInt extends Expr {
-  CompileTimeConstantInt() { compileTimeConstantInt(this, _) }
+/**
+ * Get `val` represented as type `t`, if that is possible without
+ * overflow or underflows.
+ */
+bindingset[val, t]
+private int convertIntToType(int val, IntegralType t) {
+  if t instanceof BoolType
+  then if val = 0 then result = 0 else result = 1
+  else
+    if t.isUnsigned()
+    then if val >= 0 and val.bitShiftRight(t.getSize() * 8) = 0 then result = val else none()
+    else
+      if val >= 0 and val.bitShiftRight(t.getSize() * 8 - 1) = 0
+      then result = val
+      else
+        if (-(val + 1)).bitShiftRight(t.getSize() * 8 - 1) = 0
+        then result = val
+        else none()
+}
+
+/**
+ * INTERNAL: Do not use.
+ * An expression that has been found to have an integer value at compile
+ * time.
+ */
+class CompileTimeConstantInt extends Expr {
+  int val;
+
+  CompileTimeConstantInt() { compileTimeConstantInt(this.getFullyConverted(), val) }
 
-  int getIntValue() { compileTimeConstantInt(this, result) }
+  int getIntValue() { result = val }
 }
 
 library class CompileTimeVariableExpr extends Expr {
-  CompileTimeVariableExpr() { not compileTimeConstantInt(this, _) }
+  CompileTimeVariableExpr() { not this instanceof CompileTimeConstantInt }
 }
 
 /** A helper class for evaluation of expressions. */

cpp/ql/src/semmle/code/cpp/dataflow/internal/AddressFlow.qll

@@ -29,7 +29,7 @@ private predicate stdIdentityFunction(Function f) { f.hasQualifiedName("std", ["
  */
 private predicate stdAddressOf(Function f) { f.hasQualifiedName("std", "addressof") }
 
-private predicate lvalueToLvalueStep(Expr lvalueIn, Expr lvalueOut) {
+private predicate lvalueToLvalueStepPure(Expr lvalueIn, Expr lvalueOut) {
   lvalueIn.getConversion() = lvalueOut.(ParenthesisExpr)
   or
   // When an object is implicitly converted to a reference to one of its base
@@ -42,6 +42,10 @@ private predicate lvalueToLvalueStep(Expr lvalueIn, Expr lvalueOut) {
   // such casts.
   lvalueIn.getConversion() = lvalueOut and
   lvalueOut.(CStyleCast).isImplicit()
+}
+
+private predicate lvalueToLvalueStep(Expr lvalueIn, Expr lvalueOut) {
+  lvalueToLvalueStepPure(lvalueIn, lvalueOut)
   or
   // C++ only
   lvalueIn = lvalueOut.(PrefixCrementOperation).getOperand().getFullyConverted()
@@ -214,6 +218,69 @@ private predicate referenceToUpdate(Expr reference, Expr outer, ControlFlowNode
   )
 }
 
+private predicate lvalueFromVariableAccess(VariableAccess va, Expr lvalue) {
+  // Base case for non-reference types.
+  lvalue = va and
+  not va.getConversion() instanceof ReferenceDereferenceExpr
+  or
+  // Base case for reference types where we pretend that they are
+  // non-reference types. The type of the target of `va` can be `ReferenceType`
+  // or `FunctionReferenceType`.
+  lvalue = va.getConversion().(ReferenceDereferenceExpr)
+  or
+  // lvalue -> lvalue
+  exists(Expr prev |
+    lvalueFromVariableAccess(va, prev) and
+    lvalueToLvalueStep(prev, lvalue)
+  )
+  or
+  // pointer -> lvalue
+  exists(Expr prev |
+    pointerFromVariableAccess(va, prev) and
+    pointerToLvalueStep(prev, lvalue)
+  )
+  or
+  // reference -> lvalue
+  exists(Expr prev |
+    referenceFromVariableAccess(va, prev) and
+    referenceToLvalueStep(prev, lvalue)
+  )
+}
+
+private predicate pointerFromVariableAccess(VariableAccess va, Expr pointer) {
+  // pointer -> pointer
+  exists(Expr prev |
+    pointerFromVariableAccess(va, prev) and
+    pointerToPointerStep(prev, pointer)
+  )
+  or
+  // reference -> pointer
+  exists(Expr prev |
+    referenceFromVariableAccess(va, prev) and
+    referenceToPointerStep(prev, pointer)
+  )
+  or
+  // lvalue -> pointer
+  exists(Expr prev |
+    lvalueFromVariableAccess(va, prev) and
+    lvalueToPointerStep(prev, pointer)
+  )
+}
+
+private predicate referenceFromVariableAccess(VariableAccess va, Expr reference) {
+  // reference -> reference
+  exists(Expr prev |
+    referenceFromVariableAccess(va, prev) and
+    referenceToReferenceStep(prev, reference)
+  )
+  or
+  // lvalue -> reference
+  exists(Expr prev |
+    lvalueFromVariableAccess(va, prev) and
+    lvalueToReferenceStep(prev, reference)
+  )
+}
+
 /**
  * Holds if `node` is a control-flow node that may modify `inner` (or what it
  * points to) through `outer`. The two expressions may be `Conversion`s. Plain
@@ -236,7 +303,7 @@ predicate valueToUpdate(Expr inner, Expr outer, ControlFlowNode node) {
   (
     inner instanceof VariableAccess and
     // Don't track non-field assignments
-    (assignmentTo(outer, _) implies inner instanceof FieldAccess)
+    not (assignmentTo(outer, _) and outer.(VariableAccess).getTarget() instanceof StackVariable)
     or
     inner instanceof ThisExpr
     or
@@ -245,3 +312,27 @@ predicate valueToUpdate(Expr inner, Expr outer, ControlFlowNode node) {
     // can't do anything useful with those at the moment.
   )
 }
+
+/**
+ * Holds if `e` is a fully-converted expression that evaluates to an lvalue
+ * derived from `va` and is used for reading from or assigning to. This is in
+ * contrast with a variable access that is used for taking an address (`&x`)
+ * or simply discarding its value (`x;`).
+ *
+ * This analysis does not propagate across assignments or calls, and unlike
+ * `variableAccessedAsValue` in `semmle.code.cpp.dataflow.EscapesTree` it
+ * propagates through array accesses but not field accesses. The analysis is
+ * also not concerned with whether the lvalue `e` is converted to an rvalue --
+ * to examine that, use the relevant member predicates on `Expr`.
+ *
+ * If `va` has reference type, the analysis concerns the value pointed to by
+ * the reference rather than the reference itself. The expression `e` may be a
+ * `Conversion`.
+ */
+predicate variablePartiallyAccessed(VariableAccess va, Expr e) {
+  lvalueFromVariableAccess(va, e) and
+  not lvalueToLvalueStepPure(e, _) and
+  not lvalueToPointerStep(e, _) and
+  not lvalueToReferenceStep(e, _) and
+  not e = any(ExprInVoidContext eivc | e = eivc.getConversion*())
+}

cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll

@@ -6,6 +6,7 @@ private import cpp
 private import semmle.code.cpp.dataflow.internal.FlowVar
 private import semmle.code.cpp.models.interfaces.DataFlow
 private import semmle.code.cpp.controlflow.Guards
+private import semmle.code.cpp.dataflow.internal.AddressFlow
 
 cached
 private newtype TNode =
@@ -610,6 +611,15 @@ private predicate exprToExprStep_nocfg(Expr fromExpr, Expr toExpr) {
   or
   toExpr.(AddressOfExpr).getOperand() = fromExpr
   or
+  // This rule enables flow from an array to its elements. Example: `a` to
+  // `a[i]` or `*a`, where `a` is an array type. It does not enable flow from a
+  // pointer to its indirection as in `p[i]` where `p` is a pointer type.
+  exists(Expr toConverted |
+    variablePartiallyAccessed(fromExpr, toConverted) and
+    toExpr = toConverted.getUnconverted() and
+    not toExpr = fromExpr
+  )
+  or
   toExpr.(BuiltInOperationBuiltInAddressOf).getOperand() = fromExpr
   or
   // The following case is needed to track the qualifier object for flow
@@ -629,14 +639,25 @@ private predicate exprToExprStep_nocfg(Expr fromExpr, Expr toExpr) {
   // `ClassAggregateLiteral` (`{ capture1, ..., captureN }`).
   toExpr.(LambdaExpression).getInitializer() = fromExpr
   or
+  // Data flow through a function model.
   toExpr =
     any(Call call |
-      exists(DataFlowFunction f, FunctionInput inModel, FunctionOutput outModel, int iIn |
-        call.getTarget() = f and
+      exists(DataFlowFunction f, FunctionInput inModel, FunctionOutput outModel |
         f.hasDataFlow(inModel, outModel) and
-        outModel.isReturnValue() and
-        inModel.isParameter(iIn) and
-        fromExpr = call.getArgument(iIn)
+        (
+          exists(int iIn |
+            inModel.isParameter(iIn) and
+            fromExpr = call.getArgument(iIn)
+          )
+          or
+          inModel.isQualifierObject() and
+          fromExpr = call.getQualifier()
+          or
+          inModel.isQualifierAddress() and
+          fromExpr = call.getQualifier()
+        ) and
+        call.getTarget() = f and
+        outModel.isReturnValue()
       )
     )
 }

cpp/ql/src/semmle/code/cpp/ir/dataflow/DefaultTaintTracking.qll

@@ -264,9 +264,6 @@ private predicate instructionTaintStep(Instruction i1, Instruction i2) {
     t instanceof Union
     or
     t instanceof ArrayType
-    or
-    // Buffers of unknown size
-    t instanceof UnknownType
   )
   or
   exists(BinaryInstruction bin |