diff --git a/cpp/ql/src/experimental/Likely Bugs/OverrunWriteProductFlow.ql b/cpp/ql/src/experimental/Likely Bugs/OverrunWriteProductFlow.ql index 306536ccff6..207be4071b9 100644 --- a/cpp/ql/src/experimental/Likely Bugs/OverrunWriteProductFlow.ql +++ b/cpp/ql/src/experimental/Likely Bugs/OverrunWriteProductFlow.ql @@ -1,42 +1,139 @@ /** * @name Overrunning write - * @description TODO + * @description Exceeding the size of a static array during write or access operations + * may result in a buffer overflow. * @kind path-problem * @problem.severity error * @id cpp/overrun-write * @tags reliability * security + * external/cwe/cwe-119 + * external/cwe/cwe-131 */ import cpp import experimental.semmle.code.cpp.dataflow.ProductFlow import semmle.code.cpp.ir.IR -import semmle.code.cpp.valuenumbering.GlobalValueNumbering import semmle.code.cpp.models.interfaces.Allocation import semmle.code.cpp.models.interfaces.ArrayFunction +import experimental.semmle.code.cpp.semantic.analysis.RangeAnalysis +import experimental.semmle.code.cpp.semantic.SemanticBound +import experimental.semmle.code.cpp.semantic.SemanticExprSpecific import DataFlow::PathGraph +pragma[nomagic] +Instruction getABoundIn(SemBound b, IRFunction func) { + result = b.getExpr(0) and + result.getEnclosingIRFunction() = func +} + +/** + * Holds if `i <= b + delta`. + */ +pragma[nomagic] +predicate bounded(Instruction i, Instruction b, int delta) { + exists(SemBound bound, IRFunction func | + semBounded(getSemanticExpr(i), bound, delta, true, _) and + b = getABoundIn(bound, func) and + i.getEnclosingIRFunction() = func + ) +} + +/** + * Holds if the combination of `n` and `state` represents an appropriate + * source for the expression `e` suitable for use-use flow. + */ +private predicate hasSizeImpl(Expr e, DataFlow::Node n, string state) { + // The simple case: If the size is a variable access with no qualifier we can just use the + // dataflow node for that expression and no state. + exists(VariableAccess va | + va = e and + not va instanceof FieldAccess and + n.asConvertedExpr() = va.getFullyConverted() and + state = "0" + ) + or + // If the size is a choice between two expressions we allow both to be nodes representing the size. + exists(ConditionalExpr cond | cond = e | hasSizeImpl([cond.getThen(), cond.getElse()], n, state)) + or + // If the size is an expression plus a constant, we pick the dataflow node of the expression and + // remember the constant in the state. + exists(Expr const, Expr nonconst | + e.(AddExpr).hasOperands(const, nonconst) and + state = const.getValue() and + hasSizeImpl(nonconst, n, _) + ) + or + exists(Expr const, Expr nonconst | + e.(SubExpr).hasOperands(const, nonconst) and + state = "-" + const.getValue() and + hasSizeImpl(nonconst, n, _) + ) +} + +/** + * Holds if `(n, state)` pair represents the source of flow for the size + * expression associated with `alloc`. + */ +predicate hasSize(AllocationExpr alloc, DataFlow::Node n, string state) { + hasSizeImpl(alloc.getSizeExpr(), n, state) +} + +predicate isSinkPairImpl( + CallInstruction c, DataFlow::Node bufSink, DataFlow::Node sizeSink, int delta, Expr eBuf, + Expr eSize +) { + exists(int bufIndex, int sizeIndex, Instruction sizeInstr, Instruction bufInstr | + bufInstr = bufSink.asInstruction() and + c.getArgument(bufIndex) = bufInstr and + sizeInstr = sizeSink.asInstruction() and + c.getStaticCallTarget().(ArrayFunction).hasArrayWithVariableSize(bufIndex, sizeIndex) and + bounded(c.getArgument(sizeIndex), sizeInstr, delta) and + eSize = sizeInstr.getUnconvertedResultExpression() and + eBuf = bufInstr.getUnconvertedResultExpression() and + delta >= 1 + ) +} + class StringSizeConfiguration extends ProductFlow::Configuration { StringSizeConfiguration() { this = "StringSizeConfiguration" } - override predicate isSourcePair(DataFlow::Node bufSource, DataFlow::Node sizeSource) { - bufSource.asConvertedExpr().(AllocationExpr).getSizeExpr() = sizeSource.asConvertedExpr() + override predicate isSourcePair( + DataFlow::Node bufSource, DataFlow::FlowState state1, DataFlow::Node sizeSource, + DataFlow::FlowState state2 + ) { + // In the case of an allocation like + // ```cpp + // malloc(size + 1); + // ``` + // we use `state2` to remember that there was an offset (in this case an offset of `1`) added + // to the size of the allocation. This state is then checked in `isSinkPair`. + state1 instanceof DataFlow::FlowStateEmpty and + hasSize(bufSource.asConvertedExpr(), sizeSource, state2) } - override predicate isSinkPair(DataFlow::Node bufSink, DataFlow::Node sizeSink) { - exists(CallInstruction c, int bufIndex, int sizeIndex | - c.getStaticCallTarget().(ArrayFunction).hasArrayWithVariableSize(bufIndex, sizeIndex) and - c.getArgument(bufIndex) = bufSink.asInstruction() and - c.getArgument(sizeIndex) = sizeSink.asInstruction() + override predicate isSinkPair( + DataFlow::Node bufSink, DataFlow::FlowState state1, DataFlow::Node sizeSink, + DataFlow::FlowState state2 + ) { + state1 instanceof DataFlow::FlowStateEmpty and + state2 = [0 .. 32].toString() and // An arbitrary bound because we need to bound `state2` + exists(int delta | + isSinkPairImpl(_, bufSink, sizeSink, delta, _, _) and + delta > state2.toInt() ) } } -// we don't actually check correctness yet. Right now the query just finds relevant source/sink pairs. from StringSizeConfiguration conf, DataFlow::PathNode source1, DataFlow2::PathNode source2, - DataFlow::PathNode sink1, DataFlow2::PathNode sink2 -where conf.hasFlowPath(source1, source2, sink1, sink2) -// TODO: pull delta out and display it -select sink1.getNode(), source1, sink1, "Overrunning write allocated at $@ bounded by $@.", source1, - source1.toString(), sink2, sink2.toString() + DataFlow::PathNode sink1, DataFlow2::PathNode sink2, int k, CallInstruction c, + DataFlow::Node sourceNode, Expr bound, Expr buffer, string element +where + conf.hasFlowPath(source1, source2, sink1, sink2) and + k > sink2.getState().toInt() and + isSinkPairImpl(c, sink1.getNode(), sink2.getNode(), k, buffer, bound) and + sourceNode = source1.getNode() and + if k = 1 then element = " element." else element = " elements." +select c.getUnconvertedResultExpression(), source1, sink1, + "This write may overflow $@ by " + k + element, buffer, buffer.toString() diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-119/OverrunWriteProductFlow.expected b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-119/OverrunWriteProductFlow.expected index 9bfc5ee4d01..f88b102f250 100644 --- a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-119/OverrunWriteProductFlow.expected +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-119/OverrunWriteProductFlow.expected @@ -1,52 +1,18 @@ edges -| test.cpp:16:11:16:21 | VariableAddress indirection [string] | test.cpp:24:21:24:31 | Call indirection [string] | -| test.cpp:16:11:16:21 | VariableAddress indirection [string] | test.cpp:34:21:34:31 | Call indirection [string] | | test.cpp:16:11:16:21 | VariableAddress indirection [string] | test.cpp:39:21:39:31 | Call indirection [string] | | test.cpp:18:5:18:30 | Store | test.cpp:18:10:18:15 | Load indirection [post update] [string] | | test.cpp:18:10:18:15 | Load indirection [post update] [string] | test.cpp:16:11:16:21 | VariableAddress indirection [string] | | test.cpp:18:19:18:24 | call to malloc | test.cpp:18:5:18:30 | Store | -| test.cpp:24:21:24:31 | Call indirection [string] | test.cpp:26:13:26:15 | Load indirection [string] | -| test.cpp:26:13:26:15 | Load indirection [string] | test.cpp:26:18:26:23 | FieldAddress indirection | -| test.cpp:26:18:26:23 | FieldAddress indirection | test.cpp:26:18:26:23 | Load | -| test.cpp:29:32:29:34 | str indirection [string] | test.cpp:30:13:30:15 | Load indirection [string] | -| test.cpp:30:13:30:15 | Load indirection [string] | test.cpp:30:18:30:23 | FieldAddress indirection | -| test.cpp:30:18:30:23 | FieldAddress indirection | test.cpp:30:18:30:23 | Load | -| test.cpp:34:21:34:31 | Call indirection [string] | test.cpp:35:21:35:23 | str indirection [string] | -| test.cpp:35:21:35:23 | str indirection [string] | test.cpp:29:32:29:34 | str indirection [string] | -| test.cpp:39:21:39:31 | Call indirection [string] | test.cpp:41:13:41:15 | Load indirection [string] | | test.cpp:39:21:39:31 | Call indirection [string] | test.cpp:42:13:42:15 | Load indirection [string] | -| test.cpp:39:21:39:31 | Call indirection [string] | test.cpp:44:13:44:15 | Load indirection [string] | | test.cpp:39:21:39:31 | Call indirection [string] | test.cpp:45:13:45:15 | Load indirection [string] | -| test.cpp:39:21:39:31 | Call indirection [string] | test.cpp:48:17:48:19 | Load indirection [string] | -| test.cpp:39:21:39:31 | Call indirection [string] | test.cpp:52:17:52:19 | Load indirection [string] | -| test.cpp:39:21:39:31 | Call indirection [string] | test.cpp:56:17:56:19 | Load indirection [string] | -| test.cpp:39:21:39:31 | Call indirection [string] | test.cpp:60:17:60:19 | Load indirection [string] | -| test.cpp:39:21:39:31 | Call indirection [string] | test.cpp:64:17:64:19 | Load indirection [string] | -| test.cpp:39:21:39:31 | Call indirection [string] | test.cpp:68:17:68:19 | Load indirection [string] | | test.cpp:39:21:39:31 | Call indirection [string] | test.cpp:72:17:72:19 | Load indirection [string] | | test.cpp:39:21:39:31 | Call indirection [string] | test.cpp:76:17:76:19 | Load indirection [string] | | test.cpp:39:21:39:31 | Call indirection [string] | test.cpp:80:17:80:19 | Load indirection [string] | | test.cpp:39:21:39:31 | Call indirection [string] | test.cpp:84:17:84:19 | Load indirection [string] | -| test.cpp:41:13:41:15 | Load indirection [string] | test.cpp:41:18:41:23 | FieldAddress indirection | -| test.cpp:41:18:41:23 | FieldAddress indirection | test.cpp:41:18:41:23 | Load | | test.cpp:42:13:42:15 | Load indirection [string] | test.cpp:42:18:42:23 | FieldAddress indirection | | test.cpp:42:18:42:23 | FieldAddress indirection | test.cpp:42:18:42:23 | Load | -| test.cpp:44:13:44:15 | Load indirection [string] | test.cpp:44:18:44:23 | FieldAddress indirection | -| test.cpp:44:18:44:23 | FieldAddress indirection | test.cpp:44:18:44:23 | Load | | test.cpp:45:13:45:15 | Load indirection [string] | test.cpp:45:18:45:23 | FieldAddress indirection | | test.cpp:45:18:45:23 | FieldAddress indirection | test.cpp:45:18:45:23 | Load | -| test.cpp:48:17:48:19 | Load indirection [string] | test.cpp:48:22:48:27 | FieldAddress indirection | -| test.cpp:48:22:48:27 | FieldAddress indirection | test.cpp:48:22:48:27 | Load | -| test.cpp:52:17:52:19 | Load indirection [string] | test.cpp:52:22:52:27 | FieldAddress indirection | -| test.cpp:52:22:52:27 | FieldAddress indirection | test.cpp:52:22:52:27 | Load | -| test.cpp:56:17:56:19 | Load indirection [string] | test.cpp:56:22:56:27 | FieldAddress indirection | -| test.cpp:56:22:56:27 | FieldAddress indirection | test.cpp:56:22:56:27 | Load | -| test.cpp:60:17:60:19 | Load indirection [string] | test.cpp:60:22:60:27 | FieldAddress indirection | -| test.cpp:60:22:60:27 | FieldAddress indirection | test.cpp:60:22:60:27 | Load | -| test.cpp:64:17:64:19 | Load indirection [string] | test.cpp:64:22:64:27 | FieldAddress indirection | -| test.cpp:64:22:64:27 | FieldAddress indirection | test.cpp:64:22:64:27 | Load | -| test.cpp:68:17:68:19 | Load indirection [string] | test.cpp:68:22:68:27 | FieldAddress indirection | -| test.cpp:68:22:68:27 | FieldAddress indirection | test.cpp:68:22:68:27 | Load | | test.cpp:72:17:72:19 | Load indirection [string] | test.cpp:72:22:72:27 | FieldAddress indirection | | test.cpp:72:22:72:27 | FieldAddress indirection | test.cpp:72:22:72:27 | Load | | test.cpp:76:17:76:19 | Load indirection [string] | test.cpp:76:22:76:27 | FieldAddress indirection | @@ -59,40 +25,16 @@ edges | test.cpp:90:5:90:34 | Store | test.cpp:90:10:90:15 | Load indirection [post update] [string] | | test.cpp:90:10:90:15 | Load indirection [post update] [string] | test.cpp:88:11:88:30 | VariableAddress indirection [string] | | test.cpp:90:19:90:24 | call to malloc | test.cpp:90:5:90:34 | Store | -| test.cpp:96:21:96:40 | Call indirection [string] | test.cpp:98:13:98:15 | Load indirection [string] | | test.cpp:96:21:96:40 | Call indirection [string] | test.cpp:99:13:99:15 | Load indirection [string] | -| test.cpp:96:21:96:40 | Call indirection [string] | test.cpp:101:13:101:15 | Load indirection [string] | | test.cpp:96:21:96:40 | Call indirection [string] | test.cpp:102:13:102:15 | Load indirection [string] | -| test.cpp:96:21:96:40 | Call indirection [string] | test.cpp:105:17:105:19 | Load indirection [string] | -| test.cpp:96:21:96:40 | Call indirection [string] | test.cpp:109:17:109:19 | Load indirection [string] | -| test.cpp:96:21:96:40 | Call indirection [string] | test.cpp:113:17:113:19 | Load indirection [string] | -| test.cpp:96:21:96:40 | Call indirection [string] | test.cpp:117:17:117:19 | Load indirection [string] | -| test.cpp:96:21:96:40 | Call indirection [string] | test.cpp:121:17:121:19 | Load indirection [string] | -| test.cpp:96:21:96:40 | Call indirection [string] | test.cpp:125:17:125:19 | Load indirection [string] | | test.cpp:96:21:96:40 | Call indirection [string] | test.cpp:129:17:129:19 | Load indirection [string] | | test.cpp:96:21:96:40 | Call indirection [string] | test.cpp:133:17:133:19 | Load indirection [string] | | test.cpp:96:21:96:40 | Call indirection [string] | test.cpp:137:17:137:19 | Load indirection [string] | | test.cpp:96:21:96:40 | Call indirection [string] | test.cpp:141:17:141:19 | Load indirection [string] | -| test.cpp:98:13:98:15 | Load indirection [string] | test.cpp:98:18:98:23 | FieldAddress indirection | -| test.cpp:98:18:98:23 | FieldAddress indirection | test.cpp:98:18:98:23 | Load | | test.cpp:99:13:99:15 | Load indirection [string] | test.cpp:99:18:99:23 | FieldAddress indirection | | test.cpp:99:18:99:23 | FieldAddress indirection | test.cpp:99:18:99:23 | Load | -| test.cpp:101:13:101:15 | Load indirection [string] | test.cpp:101:18:101:23 | FieldAddress indirection | -| test.cpp:101:18:101:23 | FieldAddress indirection | test.cpp:101:18:101:23 | Load | | test.cpp:102:13:102:15 | Load indirection [string] | test.cpp:102:18:102:23 | FieldAddress indirection | | test.cpp:102:18:102:23 | FieldAddress indirection | test.cpp:102:18:102:23 | Load | -| test.cpp:105:17:105:19 | Load indirection [string] | test.cpp:105:22:105:27 | FieldAddress indirection | -| test.cpp:105:22:105:27 | FieldAddress indirection | test.cpp:105:22:105:27 | Load | -| test.cpp:109:17:109:19 | Load indirection [string] | test.cpp:109:22:109:27 | FieldAddress indirection | -| test.cpp:109:22:109:27 | FieldAddress indirection | test.cpp:109:22:109:27 | Load | -| test.cpp:113:17:113:19 | Load indirection [string] | test.cpp:113:22:113:27 | FieldAddress indirection | -| test.cpp:113:22:113:27 | FieldAddress indirection | test.cpp:113:22:113:27 | Load | -| test.cpp:117:17:117:19 | Load indirection [string] | test.cpp:117:22:117:27 | FieldAddress indirection | -| test.cpp:117:22:117:27 | FieldAddress indirection | test.cpp:117:22:117:27 | Load | -| test.cpp:121:17:121:19 | Load indirection [string] | test.cpp:121:22:121:27 | FieldAddress indirection | -| test.cpp:121:22:121:27 | FieldAddress indirection | test.cpp:121:22:121:27 | Load | -| test.cpp:125:17:125:19 | Load indirection [string] | test.cpp:125:22:125:27 | FieldAddress indirection | -| test.cpp:125:22:125:27 | FieldAddress indirection | test.cpp:125:22:125:27 | Load | | test.cpp:129:17:129:19 | Load indirection [string] | test.cpp:129:22:129:27 | FieldAddress indirection | | test.cpp:129:22:129:27 | FieldAddress indirection | test.cpp:129:22:129:27 | Load | | test.cpp:133:17:133:19 | Load indirection [string] | test.cpp:133:22:133:27 | FieldAddress indirection | @@ -106,47 +48,13 @@ nodes | test.cpp:18:5:18:30 | Store | semmle.label | Store | | test.cpp:18:10:18:15 | Load indirection [post update] [string] | semmle.label | Load indirection [post update] [string] | | test.cpp:18:19:18:24 | call to malloc | semmle.label | call to malloc | -| test.cpp:24:21:24:31 | Call indirection [string] | semmle.label | Call indirection [string] | -| test.cpp:26:13:26:15 | Load indirection [string] | semmle.label | Load indirection [string] | -| test.cpp:26:18:26:23 | FieldAddress indirection | semmle.label | FieldAddress indirection | -| test.cpp:26:18:26:23 | Load | semmle.label | Load | -| test.cpp:29:32:29:34 | str indirection [string] | semmle.label | str indirection [string] | -| test.cpp:30:13:30:15 | Load indirection [string] | semmle.label | Load indirection [string] | -| test.cpp:30:18:30:23 | FieldAddress indirection | semmle.label | FieldAddress indirection | -| test.cpp:30:18:30:23 | Load | semmle.label | Load | -| test.cpp:34:21:34:31 | Call indirection [string] | semmle.label | Call indirection [string] | -| test.cpp:35:21:35:23 | str indirection [string] | semmle.label | str indirection [string] | | test.cpp:39:21:39:31 | Call indirection [string] | semmle.label | Call indirection [string] | -| test.cpp:41:13:41:15 | Load indirection [string] | semmle.label | Load indirection [string] | -| test.cpp:41:18:41:23 | FieldAddress indirection | semmle.label | FieldAddress indirection | -| test.cpp:41:18:41:23 | Load | semmle.label | Load | | test.cpp:42:13:42:15 | Load indirection [string] | semmle.label | Load indirection [string] | | test.cpp:42:18:42:23 | FieldAddress indirection | semmle.label | FieldAddress indirection | | test.cpp:42:18:42:23 | Load | semmle.label | Load | -| test.cpp:44:13:44:15 | Load indirection [string] | semmle.label | Load indirection [string] | -| test.cpp:44:18:44:23 | FieldAddress indirection | semmle.label | FieldAddress indirection | -| test.cpp:44:18:44:23 | Load | semmle.label | Load | | test.cpp:45:13:45:15 | Load indirection [string] | semmle.label | Load indirection [string] | | test.cpp:45:18:45:23 | FieldAddress indirection | semmle.label | FieldAddress indirection | | test.cpp:45:18:45:23 | Load | semmle.label | Load | -| test.cpp:48:17:48:19 | Load indirection [string] | semmle.label | Load indirection [string] | -| test.cpp:48:22:48:27 | FieldAddress indirection | semmle.label | FieldAddress indirection | -| test.cpp:48:22:48:27 | Load | semmle.label | Load | -| test.cpp:52:17:52:19 | Load indirection [string] | semmle.label | Load indirection [string] | -| test.cpp:52:22:52:27 | FieldAddress indirection | semmle.label | FieldAddress indirection | -| test.cpp:52:22:52:27 | Load | semmle.label | Load | -| test.cpp:56:17:56:19 | Load indirection [string] | semmle.label | Load indirection [string] | -| test.cpp:56:22:56:27 | FieldAddress indirection | semmle.label | FieldAddress indirection | -| test.cpp:56:22:56:27 | Load | semmle.label | Load | -| test.cpp:60:17:60:19 | Load indirection [string] | semmle.label | Load indirection [string] | -| test.cpp:60:22:60:27 | FieldAddress indirection | semmle.label | FieldAddress indirection | -| test.cpp:60:22:60:27 | Load | semmle.label | Load | -| test.cpp:64:17:64:19 | Load indirection [string] | semmle.label | Load indirection [string] | -| test.cpp:64:22:64:27 | FieldAddress indirection | semmle.label | FieldAddress indirection | -| test.cpp:64:22:64:27 | Load | semmle.label | Load | -| test.cpp:68:17:68:19 | Load indirection [string] | semmle.label | Load indirection [string] | -| test.cpp:68:22:68:27 | FieldAddress indirection | semmle.label | FieldAddress indirection | -| test.cpp:68:22:68:27 | Load | semmle.label | Load | | test.cpp:72:17:72:19 | Load indirection [string] | semmle.label | Load indirection [string] | | test.cpp:72:22:72:27 | FieldAddress indirection | semmle.label | FieldAddress indirection | | test.cpp:72:22:72:27 | Load | semmle.label | Load | @@ -164,36 +72,12 @@ nodes | test.cpp:90:10:90:15 | Load indirection [post update] [string] | semmle.label | Load indirection [post update] [string] | | test.cpp:90:19:90:24 | call to malloc | semmle.label | call to malloc | | test.cpp:96:21:96:40 | Call indirection [string] | semmle.label | Call indirection [string] | -| test.cpp:98:13:98:15 | Load indirection [string] | semmle.label | Load indirection [string] | -| test.cpp:98:18:98:23 | FieldAddress indirection | semmle.label | FieldAddress indirection | -| test.cpp:98:18:98:23 | Load | semmle.label | Load | | test.cpp:99:13:99:15 | Load indirection [string] | semmle.label | Load indirection [string] | | test.cpp:99:18:99:23 | FieldAddress indirection | semmle.label | FieldAddress indirection | | test.cpp:99:18:99:23 | Load | semmle.label | Load | -| test.cpp:101:13:101:15 | Load indirection [string] | semmle.label | Load indirection [string] | -| test.cpp:101:18:101:23 | FieldAddress indirection | semmle.label | FieldAddress indirection | -| test.cpp:101:18:101:23 | Load | semmle.label | Load | | test.cpp:102:13:102:15 | Load indirection [string] | semmle.label | Load indirection [string] | | test.cpp:102:18:102:23 | FieldAddress indirection | semmle.label | FieldAddress indirection | | test.cpp:102:18:102:23 | Load | semmle.label | Load | -| test.cpp:105:17:105:19 | Load indirection [string] | semmle.label | Load indirection [string] | -| test.cpp:105:22:105:27 | FieldAddress indirection | semmle.label | FieldAddress indirection | -| test.cpp:105:22:105:27 | Load | semmle.label | Load | -| test.cpp:109:17:109:19 | Load indirection [string] | semmle.label | Load indirection [string] | -| test.cpp:109:22:109:27 | FieldAddress indirection | semmle.label | FieldAddress indirection | -| test.cpp:109:22:109:27 | Load | semmle.label | Load | -| test.cpp:113:17:113:19 | Load indirection [string] | semmle.label | Load indirection [string] | -| test.cpp:113:22:113:27 | FieldAddress indirection | semmle.label | FieldAddress indirection | -| test.cpp:113:22:113:27 | Load | semmle.label | Load | -| test.cpp:117:17:117:19 | Load indirection [string] | semmle.label | Load indirection [string] | -| test.cpp:117:22:117:27 | FieldAddress indirection | semmle.label | FieldAddress indirection | -| test.cpp:117:22:117:27 | Load | semmle.label | Load | -| test.cpp:121:17:121:19 | Load indirection [string] | semmle.label | Load indirection [string] | -| test.cpp:121:22:121:27 | FieldAddress indirection | semmle.label | FieldAddress indirection | -| test.cpp:121:22:121:27 | Load | semmle.label | Load | -| test.cpp:125:17:125:19 | Load indirection [string] | semmle.label | Load indirection [string] | -| test.cpp:125:22:125:27 | FieldAddress indirection | semmle.label | FieldAddress indirection | -| test.cpp:125:22:125:27 | Load | semmle.label | Load | | test.cpp:129:17:129:19 | Load indirection [string] | semmle.label | Load indirection [string] | | test.cpp:129:22:129:27 | FieldAddress indirection | semmle.label | FieldAddress indirection | | test.cpp:129:22:129:27 | Load | semmle.label | Load | @@ -208,6 +92,6 @@ nodes | test.cpp:141:22:141:27 | Load | semmle.label | Load | subpaths #select -| test.cpp:26:18:26:23 | Load | test.cpp:18:19:18:24 | call to malloc | test.cpp:26:18:26:23 | Load | Overrunning write allocated at $@ bounded by $@. | test.cpp:18:19:18:24 | call to malloc | call to malloc | test.cpp:26:36:26:39 | Load | Load | -| test.cpp:30:18:30:23 | Load | test.cpp:18:19:18:24 | call to malloc | test.cpp:30:18:30:23 | Load | Overrunning write allocated at $@ bounded by $@. | test.cpp:18:19:18:24 | call to malloc | call to malloc | test.cpp:30:36:30:39 | Load | Load | -| test.cpp:41:18:41:23 | Load | test.cpp:18:19:18:24 | call to malloc | test.cpp:41:18:41:23 | Load | Overrunning write allocated at $@ bounded by $@. | test.cpp:18:19:18:24 | call to malloc | call to malloc | test.cpp:41:36:41:39 | Load | Load | +| test.cpp:42:5:42:11 | call to strncpy | test.cpp:18:19:18:24 | call to malloc | test.cpp:42:18:42:23 | Load | This write may overflow $@ by 1 element. | test.cpp:42:18:42:23 | string | string | +| test.cpp:72:9:72:15 | call to strncpy | test.cpp:18:19:18:24 | call to malloc | test.cpp:72:22:72:27 | Load | This write may overflow $@ by 1 element. | test.cpp:72:22:72:27 | string | string | +| test.cpp:80:9:80:15 | call to strncpy | test.cpp:18:19:18:24 | call to malloc | test.cpp:80:22:80:27 | Load | This write may overflow $@ by 2 elements. | test.cpp:80:22:80:27 | string | string | diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-119/test.cpp b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-119/test.cpp index b4dc7290727..d8cca6a39fc 100644 --- a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-119/test.cpp +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-119/test.cpp @@ -23,11 +23,11 @@ string_t *mk_string_t(int size) { void test1(int size, char *buf) { string_t *str = mk_string_t(size); - strncpy(str->string, buf, str->size); // GOOD [FALSE POSITIVE] + strncpy(str->string, buf, str->size); // GOOD } void strncpy_wrapper(string_t *str, char *buf) { - strncpy(str->string, buf, str->size); // GOOD [FALSE POSITIVE] + strncpy(str->string, buf, str->size); // GOOD } void test2(int size, char *buf) { @@ -38,8 +38,8 @@ void test2(int size, char *buf) { void test3(unsigned size, char *buf, unsigned anotherSize) { string_t *str = mk_string_t(size); - strncpy(str->string, buf, str->size); // GOOD [FALSE POSITIVE] - strncpy(str->string, buf, str->size + 1); // BAD [NOT DETECTED] + strncpy(str->string, buf, str->size); // GOOD + strncpy(str->string, buf, str->size + 1); // BAD strncpy(str->string, buf, size); // GOOD strncpy(str->string, buf, size + 1); // BAD [NOT DETECTED] @@ -69,7 +69,7 @@ void test3(unsigned size, char *buf, unsigned anotherSize) { } if(anotherSize <= str->size + 1) { - strncpy(str->string, buf, anotherSize); // BAD [NOT DETECTED] + strncpy(str->string, buf, anotherSize); // BAD } if(anotherSize <= size + 1) { @@ -77,7 +77,7 @@ void test3(unsigned size, char *buf, unsigned anotherSize) { } if(anotherSize <= str->size + 2) { - strncpy(str->string, buf, anotherSize); // BAD [NOT DETECTED] + strncpy(str->string, buf, anotherSize); // BAD } if(anotherSize <= size + 2) {