Merge branch 'DefaultTaintTracking-argv' into dataflow-indirect-args

Author: jbj

change-notes/1.24/analysis-javascript.md

@@ -47,6 +47,8 @@
   - [request](https://www.npmjs.com/package/request)
   - [rimraf](https://www.npmjs.com/package/rimraf)
   - [send](https://www.npmjs.com/package/send)
+  - [SockJS](https://www.npmjs.com/package/sockjs)
+  - [SockJS-client](https://www.npmjs.com/package/sockjs-client)
   - [typeahead.js](https://www.npmjs.com/package/typeahead.js)
   - [vinyl-fs](https://www.npmjs.com/package/vinyl-fs)
   - [write-file-atomic](https://www.npmjs.com/package/write-file-atomic)

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

@@ -133,10 +133,16 @@ class Function extends Declaration, ControlFlowNode, AccessHolder, @function {
    */
   Type getUnspecifiedType() { result = getType().getUnspecifiedType() }
 
-  /** Gets the nth parameter of this function. */
+  /**
+   * Gets the nth parameter of this function. There is no result for the
+   * implicit `this` parameter, and there is no `...` varargs pseudo-parameter.
+   */
   Parameter getParameter(int n) { params(unresolveElement(result), underlyingElement(this), n, _) }
 
-  /** Gets a parameter of this function. */
+  /**
+   * Gets a parameter of this function. There is no result for the implicit
+   * `this` parameter, and there is no `...` varargs pseudo-parameter.
+   */
   Parameter getAParameter() { params(unresolveElement(result), underlyingElement(this), _, _) }
 
   /**

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

@@ -60,7 +60,14 @@ private DataFlow::Node getNodeForSource(Expr source) {
   (
     result = DataFlow::exprNode(source)
     or
-    result = DataFlow::definitionByReferenceNode(source)
+    // Some of the sources in `isUserInput` are intended to match the value of
+    // an expression, while others (those modeled below) are intended to match
+    // the taint that propagates out of an argument, like the `char *` argument
+    // to `gets`. It's impossible here to tell which is which, but the "access
+    // to argv" source is definitely not intended to match an output argument,
+    // and it causes false positives if we let it.
+    result = DataFlow::definitionByReferenceNode(source) and
+    not argv(source.(VariableAccess).getTarget())
   )
 }
 

cpp/ql/src/semmle/code/cpp/ir/implementation/IRConfiguration.qll

@@ -16,6 +16,8 @@ class IRConfiguration extends TIRConfiguration {
    * Holds if IR should be created for function `func`. By default, holds for all functions.
    */
   predicate shouldCreateIRForFunction(Language::Function func) { any() }
+
+  predicate shouldEvaluateDebugStringsForFunction(Language::Function func) { any() }
 }
 
 private newtype TIREscapeAnalysisConfiguration = MkIREscapeAnalysisConfiguration()

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/IRBlock.qll

@@ -27,6 +27,9 @@ class IRBlockBase extends TIRBlock {
    * by debugging and printing code only.
    */
   int getDisplayIndex() {
+    exists(IRConfiguration::IRConfiguration config |
+      config.shouldEvaluateDebugStringsForFunction(this.getEnclosingFunction())
+    ) and
     this =
       rank[result + 1](IRBlock funcBlock |
         funcBlock.getEnclosingFunction() = getEnclosingFunction()

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll

@@ -15,6 +15,9 @@ private import Imports::OperandTag
  * `File` and line number. Used for assigning register names when printing IR.
  */
 private Instruction getAnInstructionAtLine(IRFunction irFunc, Language::File file, int line) {
+  exists(IRConfiguration::IRConfiguration config |
+    config.shouldEvaluateDebugStringsForFunction(irFunc.getFunction())
+  ) and
   exists(Language::Location location |
     irFunc = result.getEnclosingIRFunction() and
     location = result.getLocation() and
@@ -39,13 +42,20 @@ class Instruction extends Construction::TInstruction {
     result = getResultString() + " = " + getOperationString() + " " + getOperandsString()
   }
 
+  private predicate shouldGenerateDumpStrings() {
+    exists(IRConfiguration::IRConfiguration config |
+      config.shouldEvaluateDebugStringsForFunction(this.getEnclosingFunction())
+    )
+  }
+
   /**
    * Gets a string describing the operation of this instruction. This includes
    * the opcode and the immediate value, if any. For example:
    *
    * VariableAddress[x]
    */
   final string getOperationString() {
+    shouldGenerateDumpStrings() and
     if exists(getImmediateString())
     then result = getOperationPrefix() + getOpcode().toString() + "[" + getImmediateString() + "]"
     else result = getOperationPrefix() + getOpcode().toString()
@@ -57,10 +67,12 @@ class Instruction extends Construction::TInstruction {
   string getImmediateString() { none() }
 
   private string getOperationPrefix() {
+    shouldGenerateDumpStrings() and
     if this instanceof SideEffectInstruction then result = "^" else result = ""
   }
 
   private string getResultPrefix() {
+    shouldGenerateDumpStrings() and
     if getResultIRType() instanceof IRVoidType
     then result = "v"
     else
@@ -74,6 +86,7 @@ class Instruction extends Construction::TInstruction {
    * used by debugging and printing code only.
    */
   int getDisplayIndexInBlock() {
+    shouldGenerateDumpStrings() and
     exists(IRBlock block |
       this = block.getInstruction(result)
       or
@@ -87,6 +100,7 @@ class Instruction extends Construction::TInstruction {
   }
 
   private int getLineRank() {
+    shouldGenerateDumpStrings() and
     this =
       rank[result](Instruction instr |
         instr =
@@ -105,6 +119,7 @@ class Instruction extends Construction::TInstruction {
    * Example: `r1_1`
    */
   string getResultId() {
+    shouldGenerateDumpStrings() and
     result = getResultPrefix() + getAST().getLocation().getStartLine() + "_" + getLineRank()
   }
 
@@ -116,6 +131,7 @@ class Instruction extends Construction::TInstruction {
    * Example: `r1_1(int*)`
    */
   final string getResultString() {
+    shouldGenerateDumpStrings() and
     result = getResultId() + "(" + getResultLanguageType().getDumpString() + ")"
   }
 
@@ -126,6 +142,7 @@ class Instruction extends Construction::TInstruction {
    * Example: `func:r3_4, this:r3_5`
    */
   string getOperandsString() {
+    shouldGenerateDumpStrings() and
     result =
       concat(Operand operand |
         operand = getAnOperand()

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/PrintIR.qll

@@ -18,19 +18,19 @@ class PrintIRConfiguration extends TPrintIRConfiguration {
   predicate shouldPrintFunction(Language::Function func) { any() }
 }
 
-private predicate shouldPrintFunction(Language::Function func) {
-  exists(PrintIRConfiguration config | config.shouldPrintFunction(func))
-}
-
 /**
- * Override of `IRConfiguration` to only create IR for the functions that are to be dumped.
+ * Override of `IRConfiguration` to only evaluate debug strings for the functions that are to be dumped.
  */
 private class FilteredIRConfiguration extends IRConfiguration {
-  override predicate shouldCreateIRForFunction(Language::Function func) {
+  override predicate shouldEvaluateDebugStringsForFunction(Language::Function func) {
     shouldPrintFunction(func)
   }
 }
 
+private predicate shouldPrintFunction(Language::Function func) {
+  exists(PrintIRConfiguration config | config.shouldPrintFunction(func))
+}
+
 private string getAdditionalInstructionProperty(Instruction instr, string key) {
   exists(IRPropertyProvider provider | result = provider.getInstructionProperty(instr, key))
 }

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/IRInternal.qll

@@ -1,2 +1,3 @@
 import semmle.code.cpp.ir.internal.IRCppLanguage as Language
 import SSAConstruction as Construction
+import semmle.code.cpp.ir.implementation.IRConfiguration as IRConfiguration

cpp/ql/src/semmle/code/cpp/ir/implementation/raw/IRBlock.qll

@@ -27,6 +27,9 @@ class IRBlockBase extends TIRBlock {
    * by debugging and printing code only.
    */
   int getDisplayIndex() {
+    exists(IRConfiguration::IRConfiguration config |
+      config.shouldEvaluateDebugStringsForFunction(this.getEnclosingFunction())
+    ) and
     this =
       rank[result + 1](IRBlock funcBlock |
         funcBlock.getEnclosingFunction() = getEnclosingFunction()

cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Instruction.qll

@@ -15,6 +15,9 @@ private import Imports::OperandTag
  * `File` and line number. Used for assigning register names when printing IR.
  */
 private Instruction getAnInstructionAtLine(IRFunction irFunc, Language::File file, int line) {
+  exists(IRConfiguration::IRConfiguration config |
+    config.shouldEvaluateDebugStringsForFunction(irFunc.getFunction())
+  ) and
   exists(Language::Location location |
     irFunc = result.getEnclosingIRFunction() and
     location = result.getLocation() and
@@ -39,13 +42,20 @@ class Instruction extends Construction::TInstruction {
     result = getResultString() + " = " + getOperationString() + " " + getOperandsString()
   }
 
+  private predicate shouldGenerateDumpStrings() {
+    exists(IRConfiguration::IRConfiguration config |
+      config.shouldEvaluateDebugStringsForFunction(this.getEnclosingFunction())
+    )
+  }
+
   /**
    * Gets a string describing the operation of this instruction. This includes
    * the opcode and the immediate value, if any. For example:
    *
    * VariableAddress[x]
    */
   final string getOperationString() {
+    shouldGenerateDumpStrings() and
     if exists(getImmediateString())
     then result = getOperationPrefix() + getOpcode().toString() + "[" + getImmediateString() + "]"
     else result = getOperationPrefix() + getOpcode().toString()
@@ -57,10 +67,12 @@ class Instruction extends Construction::TInstruction {
   string getImmediateString() { none() }
 
   private string getOperationPrefix() {
+    shouldGenerateDumpStrings() and
     if this instanceof SideEffectInstruction then result = "^" else result = ""
   }
 
   private string getResultPrefix() {
+    shouldGenerateDumpStrings() and
     if getResultIRType() instanceof IRVoidType
     then result = "v"
     else
@@ -74,6 +86,7 @@ class Instruction extends Construction::TInstruction {
    * used by debugging and printing code only.
    */
   int getDisplayIndexInBlock() {
+    shouldGenerateDumpStrings() and
     exists(IRBlock block |
       this = block.getInstruction(result)
       or
@@ -87,6 +100,7 @@ class Instruction extends Construction::TInstruction {
   }
 
   private int getLineRank() {
+    shouldGenerateDumpStrings() and
     this =
       rank[result](Instruction instr |
         instr =
@@ -105,6 +119,7 @@ class Instruction extends Construction::TInstruction {
    * Example: `r1_1`
    */
   string getResultId() {
+    shouldGenerateDumpStrings() and
     result = getResultPrefix() + getAST().getLocation().getStartLine() + "_" + getLineRank()
   }
 
@@ -116,6 +131,7 @@ class Instruction extends Construction::TInstruction {
    * Example: `r1_1(int*)`
    */
   final string getResultString() {
+    shouldGenerateDumpStrings() and
     result = getResultId() + "(" + getResultLanguageType().getDumpString() + ")"
   }
 
@@ -126,6 +142,7 @@ class Instruction extends Construction::TInstruction {
    * Example: `func:r3_4, this:r3_5`
    */
   string getOperandsString() {
+    shouldGenerateDumpStrings() and
     result =
       concat(Operand operand |
         operand = getAnOperand()