Merge branch 'master' into get-an-assigned-value-join-order

cpp/ql/src/semmle/code/cpp/commons/Assertions.qll

@@ -12,12 +12,12 @@ abstract class Assertion extends Locatable {
 }
 
 /**
- * A libc assert, as defined in assert.h. A macro with the head
- * "assert(expr)" that expands to a conditional expression which
- * may terminate the program.
+ * A libc assert, as defined in assert.h. A macro with a head
+ * that matches the prefix "assert(", and expands to a conditional
+ * expression which may terminate the program.
  */
 class LibcAssert extends MacroInvocation, Assertion {
-  LibcAssert() { this.getMacro().getHead() = "assert(expr)" }
+  LibcAssert() { this.getMacro().getHead().matches("assert(%") }
 
   override Expr getAsserted() {
     exists(ConditionalExpr ce | this.getAGeneratedElement() = ce | result = ce.getCondition())

cpp/ql/src/semmle/code/cpp/exprs/Literal.qll

@@ -145,8 +145,6 @@ class HexLiteral extends Literal {
 
 /**
  * A C/C++ aggregate literal.
- *
- * For example:
  */
 class AggregateLiteral extends Expr, @aggregateliteral {
   override string getCanonicalQLClass() { result = "AggregateLiteral" }

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

@@ -180,12 +180,16 @@ private class ArrayContent extends Content, TArrayContent {
   override Type getType() { none() }
 }
 
-/**
- * Holds if data can flow from `node1` to `node2` via an assignment to `f`.
- * Thus, `node2` references an object with a field `f` that contains the
- * value of `node1`.
- */
-predicate storeStep(Node node1, Content f, PostUpdateNode node2) {
+private predicate storeStepNoChi(Node node1, Content f, PostUpdateNode node2) {
+  exists(FieldAddressInstruction fa, StoreInstruction store |
+    store = node2.asInstruction() and
+    store.getDestinationAddress() = fa and
+    store.getSourceValue() = node1.asInstruction() and
+    f.(FieldContent).getField() = fa.getField()
+  )
+}
+
+private predicate storeStepChi(Node node1, Content f, PostUpdateNode node2) {
   exists(FieldAddressInstruction fa, StoreInstruction store |
     node1.asInstruction() = store and
     store.getDestinationAddress() = fa and
@@ -194,6 +198,16 @@ predicate storeStep(Node node1, Content f, PostUpdateNode node2) {
   )
 }
 
+/**
+ * Holds if data can flow from `node1` to `node2` via an assignment to `f`.
+ * Thus, `node2` references an object with a field `f` that contains the
+ * value of `node1`.
+ */
+predicate storeStep(Node node1, Content f, PostUpdateNode node2) {
+  storeStepNoChi(node1, f, node2) or
+  storeStepChi(node1, f, node2)
+}
+
 /**
  * Holds if data can flow from `node1` to `node2` via a read of `f`.
  * Thus, `node1` references an object with a field `f` whose value ends up in

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

@@ -270,6 +270,25 @@ private class ExplicitFieldStoreQualifierNode extends PartialDefinitionNode {
   override Node getPreUpdateNode() { result.asInstruction() = instr.getTotal() }
 }
 
+/**
+ * Not every store instruction generates a chi instruction that we can attach a PostUpdateNode to.
+ * For instance, an update to a field of a struct containing only one field. For these cases we
+ * attach the PostUpdateNode to the store instruction. There's no obvious pre update node for this case
+ * (as the entire memory is updated), so `getPreUpdateNode` is implemented as `none()`.
+ */
+private class ExplicitSingleFieldStoreQualifierNode extends PartialDefinitionNode {
+  override StoreInstruction instr;
+
+  ExplicitSingleFieldStoreQualifierNode() {
+    exists(FieldAddressInstruction field |
+      field = instr.getDestinationAddress() and
+      not exists(ChiInstruction chi | chi.getPartial() = instr)
+    )
+  }
+
+  override Node getPreUpdateNode() { none() }
+}
+
 /**
  * A node that represents the value of a variable after a function call that
  * may have changed the variable because it's passed by reference.
@@ -404,6 +423,15 @@ predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) {
   simpleInstructionLocalFlowStep(nodeFrom.asInstruction(), nodeTo.asInstruction())
 }
 
+pragma[noinline]
+private predicate getFieldSizeOfClass(Class c, Type type, int size) {
+  exists(Field f |
+    f.getDeclaringType() = c and
+    f.getType() = type and
+    type.getSize() = size
+  )
+}
+
 cached
 private predicate simpleInstructionLocalFlowStep(Instruction iFrom, Instruction iTo) {
   iTo.(CopyInstruction).getSourceValue() = iFrom
@@ -452,6 +480,14 @@ private predicate simpleInstructionLocalFlowStep(Instruction iFrom, Instruction
     iTo.(LoadInstruction).getSourceValueOperand().getAnyDef() = chi
   )
   or
+  // Flow from stores to structs with a single field to a load of that field.
+  iTo.(LoadInstruction).getSourceValueOperand().getAnyDef() = iFrom and
+  exists(int size, Type type |
+    type = iFrom.getResultType() and
+    iTo.getResultType().getSize() = size and
+    getFieldSizeOfClass(iTo.getResultType(), type, size)
+  )
+  or
   // Flow through modeled functions
   modelFlow(iFrom, iTo)
 }

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

@@ -1,6 +1,7 @@
 import semmle.code.cpp.models.interfaces.Alias
+import semmle.code.cpp.models.interfaces.FlowSource
 
-class Fread extends AliasFunction {
+class Fread extends AliasFunction, RemoteFlowFunction {
   Fread() { this.hasGlobalName("fread") }
 
   override predicate parameterNeverEscapes(int n) {
@@ -11,4 +12,9 @@ class Fread extends AliasFunction {
   override predicate parameterEscapesOnlyViaReturn(int n) { none() }
 
   override predicate parameterIsAlwaysReturned(int n) { none() }
+
+  override predicate hasRemoteFlowSource(FunctionOutput output, string description) {
+    output.isParameterDeref(0) and
+    description = "String read by " + this.getName()
+  }
 }

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

@@ -3,12 +3,13 @@ import semmle.code.cpp.models.interfaces.Taint
 import semmle.code.cpp.models.interfaces.ArrayFunction
 import semmle.code.cpp.models.interfaces.Alias
 import semmle.code.cpp.models.interfaces.SideEffect
+import semmle.code.cpp.models.interfaces.FlowSource
 
 /**
  * The standard functions `gets` and `fgets`.
  */
 class GetsFunction extends DataFlowFunction, TaintFunction, ArrayFunction, AliasFunction,
-  SideEffectFunction {
+  SideEffectFunction, RemoteFlowFunction {
   GetsFunction() {
     exists(string name | hasGlobalOrStdName(name) |
       name = "gets" or // gets(str)
@@ -42,4 +43,9 @@ class GetsFunction extends DataFlowFunction, TaintFunction, ArrayFunction, Alias
     buffer = true and
     mustWrite = true
   }
+
+  override predicate hasRemoteFlowSource(FunctionOutput output, string description) {
+    output.isParameterDeref(0) and
+    description = "String read by " + this.getName()
+  }
 }

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

@@ -4,6 +4,7 @@ import semmle.code.cpp.models.interfaces.Taint
  * The `std::basic_string` constructor(s).
  */
 class StdStringConstructor extends TaintFunction {
+  pragma[noinline]
   StdStringConstructor() { this.hasQualifiedName("std", "basic_string", "basic_string") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {

cpp/ql/src/semmle/code/cpp/models/interfaces/FlowSource.qll

@@ -0,0 +1,21 @@
+/**
+ * Provides a class for modeling functions that return data from potentially untrusted sources. To use
+ * this QL library, create a QL class extending `DataFlowFunction` with a
+ * characteristic predicate that selects the function or set of functions you
+ * are modeling. Within that class, override the predicates provided by
+ * `RemoteFlowFunction` to match the flow within that function.
+ */
+
+import cpp
+import FunctionInputsAndOutputs
+import semmle.code.cpp.models.Models
+
+/**
+ * A library function which returns data read from a network connection.
+ */
+abstract class RemoteFlowFunction extends Function {
+  /**
+   * Holds if remote data described by `description` flows from `output` of a call to this function.
+   */
+  abstract predicate hasRemoteFlowSource(FunctionOutput output, string description);
+}

cpp/ql/src/semmle/code/cpp/security/FlowSources.qll

@@ -0,0 +1,44 @@
+/**
+ * Provides classes representing various flow sources for taint tracking.
+ */
+
+import cpp
+import semmle.code.cpp.ir.dataflow.DataFlow
+private import semmle.code.cpp.ir.IR
+import semmle.code.cpp.models.interfaces.FlowSource
+
+/** A data flow source of remote user input. */
+abstract class RemoteFlowSource extends DataFlow::Node {
+  /** Gets a string that describes the type of this remote flow source. */
+  abstract string getSourceType();
+}
+
+private class TaintedReturnSource extends RemoteFlowSource {
+  string sourceType;
+
+  TaintedReturnSource() {
+    exists(RemoteFlowFunction func, CallInstruction instr, FunctionOutput output |
+      asInstruction() = instr and
+      instr.getStaticCallTarget() = func and
+      func.hasRemoteFlowSource(output, sourceType) and
+      output.isReturnValue()
+    )
+  }
+
+  override string getSourceType() { result = sourceType }
+}
+
+private class TaintedParameterSource extends RemoteFlowSource {
+  string sourceType;
+
+  TaintedParameterSource() {
+    exists(RemoteFlowFunction func, WriteSideEffectInstruction instr, FunctionOutput output |
+      asInstruction() = instr and
+      instr.getPrimaryInstruction().(CallInstruction).getStaticCallTarget() = func and
+      func.hasRemoteFlowSource(output, sourceType) and
+      output.isParameterDeref(instr.getIndex())
+    )
+  }
+
+  override string getSourceType() { result = sourceType }
+}

cpp/ql/test/library-tests/dataflow/fields/dataflow-ir-consistency.expected

@@ -19,6 +19,7 @@ unreachableNodeCCtx
 localCallNodes
 postIsNotPre
 postHasUniquePre
+| simple.cpp:65:5:65:22 | Store | PostUpdateNode should have one pre-update node but has 0. |
 uniquePostUpdate
 postIsInSameCallable
 reverseRead

cpp/ql/test/library-tests/dataflow/fields/flow.expected

@@ -221,6 +221,10 @@ edges
 | simple.cpp:48:9:48:9 | g [b_] | simple.cpp:26:15:26:15 | f [b_] |
 | simple.cpp:51:9:51:9 | h [a_] | simple.cpp:26:15:26:15 | f [a_] |
 | simple.cpp:51:9:51:9 | h [b_] | simple.cpp:26:15:26:15 | f [b_] |
+| simple.cpp:65:5:65:5 | a [post update] [i] | simple.cpp:67:10:67:11 | a2 [i] |
+| simple.cpp:65:5:65:22 | ... = ... | simple.cpp:65:5:65:5 | a [post update] [i] |
+| simple.cpp:65:11:65:20 | call to user_input | simple.cpp:65:5:65:22 | ... = ... |
+| simple.cpp:67:10:67:11 | a2 [i] | simple.cpp:67:13:67:13 | i |
 | struct_init.c:14:24:14:25 | ab [a] | struct_init.c:15:8:15:9 | ab [a] |
 | struct_init.c:15:8:15:9 | ab [a] | struct_init.c:15:12:15:12 | a |
 | struct_init.c:20:17:20:36 | {...} [a] | struct_init.c:22:8:22:9 | ab [a] |
@@ -504,6 +508,11 @@ nodes
 | simple.cpp:48:9:48:9 | g [b_] | semmle.label | g [b_] |
 | simple.cpp:51:9:51:9 | h [a_] | semmle.label | h [a_] |
 | simple.cpp:51:9:51:9 | h [b_] | semmle.label | h [b_] |
+| simple.cpp:65:5:65:5 | a [post update] [i] | semmle.label | a [post update] [i] |
+| simple.cpp:65:5:65:22 | ... = ... | semmle.label | ... = ... |
+| simple.cpp:65:11:65:20 | call to user_input | semmle.label | call to user_input |
+| simple.cpp:67:10:67:11 | a2 [i] | semmle.label | a2 [i] |
+| simple.cpp:67:13:67:13 | i | semmle.label | i |
 | struct_init.c:14:24:14:25 | ab [a] | semmle.label | ab [a] |
 | struct_init.c:15:8:15:9 | ab [a] | semmle.label | ab [a] |
 | struct_init.c:15:12:15:12 | a | semmle.label | a |
@@ -580,6 +589,7 @@ nodes
 | simple.cpp:28:12:28:12 | call to a | simple.cpp:41:12:41:21 | call to user_input | simple.cpp:28:12:28:12 | call to a | call to a flows from $@ | simple.cpp:41:12:41:21 | call to user_input | call to user_input |
 | simple.cpp:29:12:29:12 | call to b | simple.cpp:40:12:40:21 | call to user_input | simple.cpp:29:12:29:12 | call to b | call to b flows from $@ | simple.cpp:40:12:40:21 | call to user_input | call to user_input |
 | simple.cpp:29:12:29:12 | call to b | simple.cpp:42:12:42:21 | call to user_input | simple.cpp:29:12:29:12 | call to b | call to b flows from $@ | simple.cpp:42:12:42:21 | call to user_input | call to user_input |
+| simple.cpp:67:13:67:13 | i | simple.cpp:65:11:65:20 | call to user_input | simple.cpp:67:13:67:13 | i | i flows from $@ | simple.cpp:65:11:65:20 | call to user_input | call to user_input |
 | struct_init.c:15:12:15:12 | a | struct_init.c:20:20:20:29 | call to user_input | struct_init.c:15:12:15:12 | a | a flows from $@ | struct_init.c:20:20:20:29 | call to user_input | call to user_input |
 | struct_init.c:15:12:15:12 | a | struct_init.c:27:7:27:16 | call to user_input | struct_init.c:15:12:15:12 | a | a flows from $@ | struct_init.c:27:7:27:16 | call to user_input | call to user_input |
 | struct_init.c:15:12:15:12 | a | struct_init.c:40:20:40:29 | call to user_input | struct_init.c:15:12:15:12 | a | a flows from $@ | struct_init.c:40:20:40:29 | call to user_input | call to user_input |

cpp/ql/test/library-tests/dataflow/fields/ir-flow.expected

@@ -25,6 +25,9 @@ edges
 | aliasing.cpp:79:11:79:20 | call to user_input | aliasing.cpp:80:12:80:13 | m1 |
 | aliasing.cpp:86:10:86:19 | call to user_input | aliasing.cpp:87:12:87:13 | m1 |
 | aliasing.cpp:92:12:92:21 | call to user_input | aliasing.cpp:93:12:93:13 | m1 |
+| simple.cpp:65:5:65:22 | Store [i] | simple.cpp:66:12:66:12 | Store [i] |
+| simple.cpp:65:11:65:20 | call to user_input | simple.cpp:65:5:65:22 | Store [i] |
+| simple.cpp:66:12:66:12 | Store [i] | simple.cpp:67:13:67:13 | i |
 | struct_init.c:20:20:20:29 | call to user_input | struct_init.c:22:11:22:11 | a |
 | struct_init.c:27:7:27:16 | call to user_input | struct_init.c:31:23:31:23 | a |
 nodes
@@ -63,6 +66,10 @@ nodes
 | aliasing.cpp:87:12:87:13 | m1 | semmle.label | m1 |
 | aliasing.cpp:92:12:92:21 | call to user_input | semmle.label | call to user_input |
 | aliasing.cpp:93:12:93:13 | m1 | semmle.label | m1 |
+| simple.cpp:65:5:65:22 | Store [i] | semmle.label | Store [i] |
+| simple.cpp:65:11:65:20 | call to user_input | semmle.label | call to user_input |
+| simple.cpp:66:12:66:12 | Store [i] | semmle.label | Store [i] |
+| simple.cpp:67:13:67:13 | i | semmle.label | i |
 | struct_init.c:20:20:20:29 | call to user_input | semmle.label | call to user_input |
 | struct_init.c:22:11:22:11 | a | semmle.label | a |
 | struct_init.c:27:7:27:16 | call to user_input | semmle.label | call to user_input |
@@ -78,5 +85,6 @@ nodes
 | aliasing.cpp:80:12:80:13 | m1 | aliasing.cpp:79:11:79:20 | call to user_input | aliasing.cpp:80:12:80:13 | m1 | m1 flows from $@ | aliasing.cpp:79:11:79:20 | call to user_input | call to user_input |
 | aliasing.cpp:87:12:87:13 | m1 | aliasing.cpp:86:10:86:19 | call to user_input | aliasing.cpp:87:12:87:13 | m1 | m1 flows from $@ | aliasing.cpp:86:10:86:19 | call to user_input | call to user_input |
 | aliasing.cpp:93:12:93:13 | m1 | aliasing.cpp:92:12:92:21 | call to user_input | aliasing.cpp:93:12:93:13 | m1 | m1 flows from $@ | aliasing.cpp:92:12:92:21 | call to user_input | call to user_input |
+| simple.cpp:67:13:67:13 | i | simple.cpp:65:11:65:20 | call to user_input | simple.cpp:67:13:67:13 | i | i flows from $@ | simple.cpp:65:11:65:20 | call to user_input | call to user_input |
 | struct_init.c:22:11:22:11 | a | struct_init.c:20:20:20:29 | call to user_input | struct_init.c:22:11:22:11 | a | a flows from $@ | struct_init.c:20:20:20:29 | call to user_input | call to user_input |
 | struct_init.c:31:23:31:23 | a | struct_init.c:27:7:27:16 | call to user_input | struct_init.c:31:23:31:23 | a | a flows from $@ | struct_init.c:27:7:27:16 | call to user_input | call to user_input |

cpp/ql/test/library-tests/dataflow/fields/simple.cpp

@@ -53,4 +53,18 @@ void foo()
     // Nothing should alert
     bar(i);
 }
+
+struct A
+{
+    int i;
+};
+
+void single_field_test()
+{
+    A a;
+    a.i = user_input();
+    A a2 = a;
+    sink(a2.i);
+}
+
 } // namespace Simple

cpp/ql/test/library-tests/syntax-zoo/dataflow-ir-consistency.expected

@@ -788,6 +788,10 @@ unreachableNodeCCtx
 localCallNodes
 postIsNotPre
 postHasUniquePre
+| assignexpr.cpp:9:2:9:12 | Store | PostUpdateNode should have one pre-update node but has 0. |
+| bad_asts.cpp:15:10:15:12 | Store | PostUpdateNode should have one pre-update node but has 0. |
+| file://:0:0:0:0 | Store | PostUpdateNode should have one pre-update node but has 0. |
+| ir.cpp:531:14:531:14 | Store | PostUpdateNode should have one pre-update node but has 0. |
 uniquePostUpdate
 postIsInSameCallable
 reverseRead