diff --git a/cpp/ql/src/Security/CWE/CWE-190/ArithmeticTainted.ql b/cpp/ql/src/Security/CWE/CWE-190/ArithmeticTainted.ql index 1981836a923..810039f4e1a 100644 --- a/cpp/ql/src/Security/CWE/CWE-190/ArithmeticTainted.ql +++ b/cpp/ql/src/Security/CWE/CWE-190/ArithmeticTainted.ql @@ -14,10 +14,13 @@ import cpp import semmle.code.cpp.security.Overflow -import semmle.code.cpp.security.Security -import semmle.code.cpp.ir.dataflow.internal.DefaultTaintTrackingImpl -import TaintedWithPath +import semmle.code.cpp.dataflow.new.TaintTracking +import semmle.code.cpp.dataflow.new.DataFlow +import semmle.code.cpp.ir.IR +import semmle.code.cpp.controlflow.IRGuards as IRGuards +import semmle.code.cpp.security.FlowSources as FS import Bounded +import Flow::PathGraph bindingset[op] predicate missingGuard(Operation op, Expr e, string effect) { @@ -28,28 +31,90 @@ predicate missingGuard(Operation op, Expr e, string effect) { not e instanceof VariableAccess and effect = "overflow" } -class Configuration extends TaintTrackingConfiguration { - override predicate isSink(Element e) { - exists(Operation op | - missingGuard(op, e, _) and - op.getAnOperand() = e - | - op instanceof UnaryArithmeticOperation or - op instanceof BinaryArithmeticOperation or - op instanceof AssignArithmeticOperation - ) - } +predicate isSource(FS::FlowSource source, string sourceType) { sourceType = source.getSourceType() } - override predicate isBarrier(Expr e) { - super.isBarrier(e) or bounded(e) or e.getUnspecifiedType().(IntegralType).getSize() <= 1 +predicate isSink(DataFlow::Node sink, Operation op, Expr e) { + e = sink.asExpr() and + missingGuard(op, e, _) and + op.getAnOperand() = e and + ( + op instanceof UnaryArithmeticOperation or + op instanceof BinaryArithmeticOperation or + op instanceof AssignArithmeticOperation + ) +} + +predicate hasUpperBoundsCheck(Variable var) { + exists(RelationalOperation oper, VariableAccess access | + oper.getAnOperand() = access and + access.getTarget() = var and + // Comparing to 0 is not an upper bound check + not oper.getAnOperand().getValue() = "0" + ) +} + +predicate constantInstruction(Instruction instr) { + instr instanceof ConstantInstruction or + constantInstruction(instr.(UnaryInstruction).getUnary()) +} + +predicate readsVariable(LoadInstruction load, Variable var) { + load.getSourceAddress().(VariableAddressInstruction).getAstVariable() = var +} + +predicate nodeIsBarrierEqualityCandidate(DataFlow::Node node, Operand access, Variable checkedVar) { + exists(Instruction instr | instr = node.asInstruction() | + readsVariable(instr, checkedVar) and + any(IRGuards::IRGuardCondition guard).ensuresEq(access, _, _, instr.getBlock(), true) + ) +} + +module Config implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { isSource(source, _) } + + predicate isSink(DataFlow::Node sink) { isSink(sink, _, _) } + + predicate isBarrier(DataFlow::Node node) { + exists(StoreInstruction store | store = node.asInstruction() | + // Block flow to "likely small expressions" + bounded(store.getSourceValue().getUnconvertedResultExpression()) + or + // Block flow to "small types" + store.getResultType().getUnspecifiedType().(IntegralType).getSize() <= 1 + ) + or + // Block flow if there's an upper bound check of the variable anywhere in the program + exists(Variable checkedVar, Instruction instr | instr = node.asInstruction() | + readsVariable(instr, checkedVar) and + hasUpperBoundsCheck(checkedVar) + ) + or + // Block flow if the node is guarded by an equality check + exists(Variable checkedVar, Operand access | + nodeIsBarrierEqualityCandidate(node, access, checkedVar) and + readsVariable(access.getDef(), checkedVar) + ) + or + // Block flow to any binary instruction whose operands are both non-constants. + exists(BinaryInstruction iTo | + iTo = node.asInstruction() and + not constantInstruction(iTo.getLeft()) and + not constantInstruction(iTo.getRight()) and + // propagate taint from either the pointer or the offset, regardless of constantness + not iTo instanceof PointerArithmeticInstruction + ) } } -from Expr origin, Expr e, string effect, PathNode sourceNode, PathNode sinkNode, Operation op +module Flow = TaintTracking::Global; + +from + Expr e, string effect, Flow::PathNode source, Flow::PathNode sink, Operation op, string sourceType where - taintedWithPath(origin, e, sourceNode, sinkNode) and - op.getAnOperand() = e and + Flow::flowPath(source, sink) and + isSource(source.getNode(), sourceType) and + isSink(sink.getNode(), op, e) and missingGuard(op, e, effect) -select e, sourceNode, sinkNode, +select e, source, sink, "$@ flows to an operand of an arithmetic expression, potentially causing an " + effect + ".", - origin, "User-provided value" + source, sourceType diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-190/SAMATE/ArithmeticTainted.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-190/SAMATE/ArithmeticTainted.expected index b86b85a91eb..d266afbe445 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-190/SAMATE/ArithmeticTainted.expected +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-190/SAMATE/ArithmeticTainted.expected @@ -1,13 +1,8 @@ edges -| examples.cpp:63:26:63:30 | & ... | examples.cpp:66:11:66:14 | data | -| examples.cpp:63:26:63:30 | & ... | examples.cpp:66:11:66:14 | data | | examples.cpp:63:26:63:30 | fscanf output argument | examples.cpp:66:11:66:14 | data | -| examples.cpp:63:26:63:30 | fscanf output argument | examples.cpp:66:11:66:14 | data | -subpaths nodes -| examples.cpp:63:26:63:30 | & ... | semmle.label | & ... | | examples.cpp:63:26:63:30 | fscanf output argument | semmle.label | fscanf output argument | | examples.cpp:66:11:66:14 | data | semmle.label | data | -| examples.cpp:66:11:66:14 | data | semmle.label | data | +subpaths #select -| examples.cpp:66:11:66:14 | data | examples.cpp:63:26:63:30 | & ... | examples.cpp:66:11:66:14 | data | $@ flows to an operand of an arithmetic expression, potentially causing an underflow. | examples.cpp:63:26:63:30 | & ... | User-provided value | +| examples.cpp:66:11:66:14 | data | examples.cpp:63:26:63:30 | fscanf output argument | examples.cpp:66:11:66:14 | data | $@ flows to an operand of an arithmetic expression, potentially causing an underflow. | examples.cpp:63:26:63:30 | fscanf output argument | value read by fscanf | diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/tainted/ArithmeticTainted.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/tainted/ArithmeticTainted.expected index 9b632194f4a..f3edc87450e 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/tainted/ArithmeticTainted.expected +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/tainted/ArithmeticTainted.expected @@ -1,86 +1,59 @@ edges | test2.cpp:12:21:12:21 | v | test2.cpp:14:11:14:11 | v | -| test2.cpp:12:21:12:21 | v | test2.cpp:14:11:14:11 | v | -| test2.cpp:25:22:25:23 | & ... | test2.cpp:27:13:27:13 | v | | test2.cpp:25:22:25:23 | fscanf output argument | test2.cpp:27:13:27:13 | v | | test2.cpp:27:13:27:13 | v | test2.cpp:12:21:12:21 | v | -| test2.cpp:36:9:36:14 | buffer | test2.cpp:39:9:39:11 | num | -| test2.cpp:36:9:36:14 | buffer | test2.cpp:39:9:39:11 | num | -| test2.cpp:36:9:36:14 | buffer | test2.cpp:39:9:39:11 | num | -| test2.cpp:36:9:36:14 | buffer | test2.cpp:39:9:39:11 | num | -| test2.cpp:36:9:36:14 | buffer | test2.cpp:40:3:40:5 | num | -| test2.cpp:36:9:36:14 | buffer | test2.cpp:40:3:40:5 | num | -| test2.cpp:36:9:36:14 | buffer | test2.cpp:40:3:40:5 | num | -| test2.cpp:36:9:36:14 | buffer | test2.cpp:40:3:40:5 | num | -| test2.cpp:36:9:36:14 | fgets output argument | test2.cpp:39:9:39:11 | num | | test2.cpp:36:9:36:14 | fgets output argument | test2.cpp:39:9:39:11 | num | | test2.cpp:36:9:36:14 | fgets output argument | test2.cpp:40:3:40:5 | num | -| test2.cpp:36:9:36:14 | fgets output argument | test2.cpp:40:3:40:5 | num | -| test5.cpp:5:5:5:17 | getTaintedInt indirection | test5.cpp:17:6:17:18 | call to getTaintedInt | +| test3.c:10:27:10:30 | argv indirection | test.c:14:15:14:28 | maxConnections | +| test3.c:10:27:10:30 | argv indirection | test.c:44:7:44:10 | len2 | +| test3.c:10:27:10:30 | argv indirection | test.c:54:7:54:10 | len3 | | test5.cpp:5:5:5:17 | getTaintedInt indirection | test5.cpp:17:6:17:18 | call to getTaintedInt | | test5.cpp:5:5:5:17 | getTaintedInt indirection | test5.cpp:18:6:18:18 | call to getTaintedInt | -| test5.cpp:9:7:9:9 | buf | test5.cpp:5:5:5:17 | getTaintedInt indirection | -| test5.cpp:9:7:9:9 | buf | test5.cpp:5:5:5:17 | getTaintedInt indirection | | test5.cpp:9:7:9:9 | gets output argument | test5.cpp:5:5:5:17 | getTaintedInt indirection | | test5.cpp:18:6:18:18 | call to getTaintedInt | test5.cpp:19:6:19:6 | y | -| test5.cpp:18:6:18:18 | call to getTaintedInt | test5.cpp:19:6:19:6 | y | -| test.c:11:29:11:32 | argv | test.c:14:15:14:28 | maxConnections | -| test.c:11:29:11:32 | argv | test.c:14:15:14:28 | maxConnections | -| test.c:11:29:11:32 | argv | test.c:14:15:14:28 | maxConnections | -| test.c:11:29:11:32 | argv | test.c:14:15:14:28 | maxConnections | -| test.c:41:17:41:20 | argv | test.c:44:7:44:10 | len2 | -| test.c:41:17:41:20 | argv | test.c:44:7:44:10 | len2 | -| test.c:41:17:41:20 | argv | test.c:44:7:44:10 | len2 | -| test.c:41:17:41:20 | argv | test.c:44:7:44:10 | len2 | -| test.c:51:17:51:20 | argv | test.c:54:7:54:10 | len3 | -| test.c:51:17:51:20 | argv | test.c:54:7:54:10 | len3 | -| test.c:51:17:51:20 | argv | test.c:54:7:54:10 | len3 | -| test.c:51:17:51:20 | argv | test.c:54:7:54:10 | len3 | -subpaths +| test.c:10:27:10:30 | argv indirection | test.c:14:15:14:28 | maxConnections | +| test.c:10:27:10:30 | argv indirection | test.c:44:7:44:10 | len2 | +| test.c:10:27:10:30 | argv indirection | test.c:54:7:54:10 | len3 | nodes | test2.cpp:12:21:12:21 | v | semmle.label | v | | test2.cpp:14:11:14:11 | v | semmle.label | v | -| test2.cpp:14:11:14:11 | v | semmle.label | v | -| test2.cpp:25:22:25:23 | & ... | semmle.label | & ... | | test2.cpp:25:22:25:23 | fscanf output argument | semmle.label | fscanf output argument | | test2.cpp:27:13:27:13 | v | semmle.label | v | -| test2.cpp:36:9:36:14 | buffer | semmle.label | buffer | -| test2.cpp:36:9:36:14 | buffer | semmle.label | buffer | | test2.cpp:36:9:36:14 | fgets output argument | semmle.label | fgets output argument | | test2.cpp:39:9:39:11 | num | semmle.label | num | -| test2.cpp:39:9:39:11 | num | semmle.label | num | -| test2.cpp:40:3:40:5 | num | semmle.label | num | | test2.cpp:40:3:40:5 | num | semmle.label | num | +| test3.c:10:27:10:30 | argv indirection | semmle.label | argv indirection | | test5.cpp:5:5:5:17 | getTaintedInt indirection | semmle.label | getTaintedInt indirection | -| test5.cpp:9:7:9:9 | buf | semmle.label | buf | -| test5.cpp:9:7:9:9 | buf | semmle.label | buf | | test5.cpp:9:7:9:9 | gets output argument | semmle.label | gets output argument | | test5.cpp:17:6:17:18 | call to getTaintedInt | semmle.label | call to getTaintedInt | -| test5.cpp:17:6:17:18 | call to getTaintedInt | semmle.label | call to getTaintedInt | | test5.cpp:18:6:18:18 | call to getTaintedInt | semmle.label | call to getTaintedInt | | test5.cpp:19:6:19:6 | y | semmle.label | y | -| test5.cpp:19:6:19:6 | y | semmle.label | y | -| test.c:11:29:11:32 | argv | semmle.label | argv | -| test.c:11:29:11:32 | argv | semmle.label | argv | +| test.c:10:27:10:30 | argv indirection | semmle.label | argv indirection | | test.c:14:15:14:28 | maxConnections | semmle.label | maxConnections | -| test.c:14:15:14:28 | maxConnections | semmle.label | maxConnections | -| test.c:41:17:41:20 | argv | semmle.label | argv | -| test.c:41:17:41:20 | argv | semmle.label | argv | | test.c:44:7:44:10 | len2 | semmle.label | len2 | -| test.c:44:7:44:10 | len2 | semmle.label | len2 | -| test.c:51:17:51:20 | argv | semmle.label | argv | -| test.c:51:17:51:20 | argv | semmle.label | argv | -| test.c:54:7:54:10 | len3 | semmle.label | len3 | | test.c:54:7:54:10 | len3 | semmle.label | len3 | +subpaths #select -| test2.cpp:14:11:14:11 | v | test2.cpp:25:22:25:23 | & ... | test2.cpp:14:11:14:11 | v | $@ flows to an operand of an arithmetic expression, potentially causing an overflow. | test2.cpp:25:22:25:23 | & ... | User-provided value | -| test2.cpp:14:11:14:11 | v | test2.cpp:25:22:25:23 | & ... | test2.cpp:14:11:14:11 | v | $@ flows to an operand of an arithmetic expression, potentially causing an underflow. | test2.cpp:25:22:25:23 | & ... | User-provided value | -| test2.cpp:39:9:39:11 | num | test2.cpp:36:9:36:14 | buffer | test2.cpp:39:9:39:11 | num | $@ flows to an operand of an arithmetic expression, potentially causing an overflow. | test2.cpp:36:9:36:14 | buffer | User-provided value | -| test2.cpp:40:3:40:5 | num | test2.cpp:36:9:36:14 | buffer | test2.cpp:40:3:40:5 | num | $@ flows to an operand of an arithmetic expression, potentially causing an overflow. | test2.cpp:36:9:36:14 | buffer | User-provided value | -| test5.cpp:17:6:17:18 | call to getTaintedInt | test5.cpp:9:7:9:9 | buf | test5.cpp:17:6:17:18 | call to getTaintedInt | $@ flows to an operand of an arithmetic expression, potentially causing an overflow. | test5.cpp:9:7:9:9 | buf | User-provided value | -| test5.cpp:19:6:19:6 | y | test5.cpp:9:7:9:9 | buf | test5.cpp:19:6:19:6 | y | $@ flows to an operand of an arithmetic expression, potentially causing an overflow. | test5.cpp:9:7:9:9 | buf | User-provided value | -| test5.cpp:19:6:19:6 | y | test5.cpp:9:7:9:9 | buf | test5.cpp:19:6:19:6 | y | $@ flows to an operand of an arithmetic expression, potentially causing an underflow. | test5.cpp:9:7:9:9 | buf | User-provided value | -| test.c:14:15:14:28 | maxConnections | test.c:11:29:11:32 | argv | test.c:14:15:14:28 | maxConnections | $@ flows to an operand of an arithmetic expression, potentially causing an overflow. | test.c:11:29:11:32 | argv | User-provided value | -| test.c:14:15:14:28 | maxConnections | test.c:11:29:11:32 | argv | test.c:14:15:14:28 | maxConnections | $@ flows to an operand of an arithmetic expression, potentially causing an underflow. | test.c:11:29:11:32 | argv | User-provided value | -| test.c:44:7:44:10 | len2 | test.c:41:17:41:20 | argv | test.c:44:7:44:10 | len2 | $@ flows to an operand of an arithmetic expression, potentially causing an underflow. | test.c:41:17:41:20 | argv | User-provided value | -| test.c:54:7:54:10 | len3 | test.c:51:17:51:20 | argv | test.c:54:7:54:10 | len3 | $@ flows to an operand of an arithmetic expression, potentially causing an underflow. | test.c:51:17:51:20 | argv | User-provided value | +| test2.cpp:14:11:14:11 | v | test2.cpp:25:22:25:23 | fscanf output argument | test2.cpp:14:11:14:11 | v | $@ flows to an operand of an arithmetic expression, potentially causing an overflow. | test2.cpp:25:22:25:23 | fscanf output argument | value read by fscanf | +| test2.cpp:14:11:14:11 | v | test2.cpp:25:22:25:23 | fscanf output argument | test2.cpp:14:11:14:11 | v | $@ flows to an operand of an arithmetic expression, potentially causing an underflow. | test2.cpp:25:22:25:23 | fscanf output argument | value read by fscanf | +| test2.cpp:39:9:39:11 | num | test2.cpp:36:9:36:14 | fgets output argument | test2.cpp:39:9:39:11 | num | $@ flows to an operand of an arithmetic expression, potentially causing an overflow. | test2.cpp:36:9:36:14 | fgets output argument | string read by fgets | +| test2.cpp:40:3:40:5 | num | test2.cpp:36:9:36:14 | fgets output argument | test2.cpp:40:3:40:5 | num | $@ flows to an operand of an arithmetic expression, potentially causing an overflow. | test2.cpp:36:9:36:14 | fgets output argument | string read by fgets | +| test5.cpp:17:6:17:18 | call to getTaintedInt | test5.cpp:9:7:9:9 | gets output argument | test5.cpp:17:6:17:18 | call to getTaintedInt | $@ flows to an operand of an arithmetic expression, potentially causing an overflow. | test5.cpp:9:7:9:9 | gets output argument | string read by gets | +| test5.cpp:19:6:19:6 | y | test5.cpp:9:7:9:9 | gets output argument | test5.cpp:19:6:19:6 | y | $@ flows to an operand of an arithmetic expression, potentially causing an overflow. | test5.cpp:9:7:9:9 | gets output argument | string read by gets | +| test5.cpp:19:6:19:6 | y | test5.cpp:9:7:9:9 | gets output argument | test5.cpp:19:6:19:6 | y | $@ flows to an operand of an arithmetic expression, potentially causing an underflow. | test5.cpp:9:7:9:9 | gets output argument | string read by gets | +| test.c:14:15:14:28 | maxConnections | test3.c:10:27:10:30 | argv indirection | test.c:14:15:14:28 | maxConnections | $@ flows to an operand of an arithmetic expression, potentially causing an overflow. | test3.c:10:27:10:30 | argv indirection | a command-line argument | +| test.c:14:15:14:28 | maxConnections | test3.c:10:27:10:30 | argv indirection | test.c:14:15:14:28 | maxConnections | $@ flows to an operand of an arithmetic expression, potentially causing an overflow. | test.c:10:27:10:30 | argv indirection | a command-line argument | +| test.c:14:15:14:28 | maxConnections | test3.c:10:27:10:30 | argv indirection | test.c:14:15:14:28 | maxConnections | $@ flows to an operand of an arithmetic expression, potentially causing an underflow. | test3.c:10:27:10:30 | argv indirection | a command-line argument | +| test.c:14:15:14:28 | maxConnections | test3.c:10:27:10:30 | argv indirection | test.c:14:15:14:28 | maxConnections | $@ flows to an operand of an arithmetic expression, potentially causing an underflow. | test.c:10:27:10:30 | argv indirection | a command-line argument | +| test.c:14:15:14:28 | maxConnections | test.c:10:27:10:30 | argv indirection | test.c:14:15:14:28 | maxConnections | $@ flows to an operand of an arithmetic expression, potentially causing an overflow. | test3.c:10:27:10:30 | argv indirection | a command-line argument | +| test.c:14:15:14:28 | maxConnections | test.c:10:27:10:30 | argv indirection | test.c:14:15:14:28 | maxConnections | $@ flows to an operand of an arithmetic expression, potentially causing an overflow. | test.c:10:27:10:30 | argv indirection | a command-line argument | +| test.c:14:15:14:28 | maxConnections | test.c:10:27:10:30 | argv indirection | test.c:14:15:14:28 | maxConnections | $@ flows to an operand of an arithmetic expression, potentially causing an underflow. | test3.c:10:27:10:30 | argv indirection | a command-line argument | +| test.c:14:15:14:28 | maxConnections | test.c:10:27:10:30 | argv indirection | test.c:14:15:14:28 | maxConnections | $@ flows to an operand of an arithmetic expression, potentially causing an underflow. | test.c:10:27:10:30 | argv indirection | a command-line argument | +| test.c:44:7:44:10 | len2 | test3.c:10:27:10:30 | argv indirection | test.c:44:7:44:10 | len2 | $@ flows to an operand of an arithmetic expression, potentially causing an underflow. | test3.c:10:27:10:30 | argv indirection | a command-line argument | +| test.c:44:7:44:10 | len2 | test3.c:10:27:10:30 | argv indirection | test.c:44:7:44:10 | len2 | $@ flows to an operand of an arithmetic expression, potentially causing an underflow. | test.c:10:27:10:30 | argv indirection | a command-line argument | +| test.c:44:7:44:10 | len2 | test.c:10:27:10:30 | argv indirection | test.c:44:7:44:10 | len2 | $@ flows to an operand of an arithmetic expression, potentially causing an underflow. | test3.c:10:27:10:30 | argv indirection | a command-line argument | +| test.c:44:7:44:10 | len2 | test.c:10:27:10:30 | argv indirection | test.c:44:7:44:10 | len2 | $@ flows to an operand of an arithmetic expression, potentially causing an underflow. | test.c:10:27:10:30 | argv indirection | a command-line argument | +| test.c:54:7:54:10 | len3 | test3.c:10:27:10:30 | argv indirection | test.c:54:7:54:10 | len3 | $@ flows to an operand of an arithmetic expression, potentially causing an underflow. | test3.c:10:27:10:30 | argv indirection | a command-line argument | +| test.c:54:7:54:10 | len3 | test3.c:10:27:10:30 | argv indirection | test.c:54:7:54:10 | len3 | $@ flows to an operand of an arithmetic expression, potentially causing an underflow. | test.c:10:27:10:30 | argv indirection | a command-line argument | +| test.c:54:7:54:10 | len3 | test.c:10:27:10:30 | argv indirection | test.c:54:7:54:10 | len3 | $@ flows to an operand of an arithmetic expression, potentially causing an underflow. | test3.c:10:27:10:30 | argv indirection | a command-line argument | +| test.c:54:7:54:10 | len3 | test.c:10:27:10:30 | argv indirection | test.c:54:7:54:10 | len3 | $@ flows to an operand of an arithmetic expression, potentially causing an underflow. | test.c:10:27:10:30 | argv indirection | a command-line argument |