diff --git a/cpp/ql/src/Security/CWE/CWE-134/UncontrolledFormatStringThroughGlobalVar.ql b/cpp/ql/src/Security/CWE/CWE-134/UncontrolledFormatStringThroughGlobalVar.ql index f3cb4fcf1bb..88341f7ca9d 100644 --- a/cpp/ql/src/Security/CWE/CWE-134/UncontrolledFormatStringThroughGlobalVar.ql +++ b/cpp/ql/src/Security/CWE/CWE-134/UncontrolledFormatStringThroughGlobalVar.ql @@ -3,7 +3,7 @@ * @description Using externally-controlled format strings in * printf-style functions can lead to buffer overflows * or data representation problems. - * @kind problem + * @kind path-problem * @problem.severity warning * @precision medium * @id cpp/tainted-format-string-through-global @@ -16,15 +16,24 @@ import cpp import semmle.code.cpp.security.FunctionWithWrappers import semmle.code.cpp.security.Security import semmle.code.cpp.security.TaintTracking +import TaintedWithPath + +class Configuration extends TaintTrackingConfiguration { + override predicate isSink(Element tainted) { + exists(PrintfLikeFunction printf | printf.outermostWrapperFunctionCall(tainted, _)) + } + + override predicate taintThroughGlobals() { any() } +} from - PrintfLikeFunction printf, Expr arg, string printfFunction, Expr userValue, string cause, - string globalVar + PrintfLikeFunction printf, Expr arg, PathNode sourceNode, PathNode sinkNode, + string printfFunction, Expr userValue, string cause where printf.outermostWrapperFunctionCall(arg, printfFunction) and - not tainted(_, arg) and - taintedIncludingGlobalVars(userValue, arg, globalVar) and + not taintedWithoutGlobals(arg) and + taintedWithPath(userValue, arg, sourceNode, sinkNode) and isUserInput(userValue, cause) select arg, - "This value may flow through $@, originating from $@, and is a formatting argument to " + - printfFunction + ".", globalVarFromId(globalVar), globalVar, userValue, cause + "The value of this argument may come from $@ and is being used as a formatting argument to " + + printfFunction, userValue, cause diff --git a/cpp/ql/src/semmle/code/cpp/ir/dataflow/DefaultTaintTracking.qll b/cpp/ql/src/semmle/code/cpp/ir/dataflow/DefaultTaintTracking.qll index 498449bdd71..c291246a774 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/dataflow/DefaultTaintTracking.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/dataflow/DefaultTaintTracking.qll @@ -407,6 +407,12 @@ module TaintedWithPath { /** Override this to specify which elements are sinks in this configuration. */ abstract predicate isSink(Element e); + /** + * Override this predicate to `any()` to allow taint to flow through global + * variables. + */ + predicate taintThroughGlobals() { none() } + /** Gets a textual representation of this element. */ string toString() { result = "TaintTrackingConfiguration" } } @@ -422,6 +428,12 @@ module TaintedWithPath { override predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) { instructionTaintStep(n1.asInstruction(), n2.asInstruction()) + or + exists(TaintTrackingConfiguration cfg | cfg.taintThroughGlobals() | + writesVariable(n1.asInstruction(), n2.asVariable().(GlobalOrNamespaceVariable)) + or + readsVariable(n2.asInstruction(), n1.asVariable().(GlobalOrNamespaceVariable)) + ) } override predicate isBarrier(DataFlow::Node node) { nodeIsBarrier(node) } @@ -564,4 +576,26 @@ module TaintedWithPath { tainted = sinkNode.(FinalPathNode).inner() ) } + + private predicate isGlobalVariablePathNode(WrapPathNode n) { + n.inner().getNode().asVariable() instanceof GlobalOrNamespaceVariable + } + + private predicate edgesWithoutGlobals(PathNode a, PathNode b) { + edges(a, b) and + not isGlobalVariablePathNode(a) and + not isGlobalVariablePathNode(b) + } + + /** + * Holds if `tainted` can be reached from a taint source without passing + * through a global variable. + */ + predicate taintedWithoutGlobals(Element tainted) { + exists(PathNode sourceNode, FinalPathNode sinkNode | + sourceNode.(WrapPathNode).inner().getNode() = getNodeForSource(_) and + edgesWithoutGlobals+(sourceNode, sinkNode) and + tainted = sinkNode.inner() + ) + } } diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-134/semmle/globalVars/UncontrolledFormatStringThroughGlobalVar.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-134/semmle/globalVars/UncontrolledFormatStringThroughGlobalVar.expected index 0f29313da8e..263399b2af8 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-134/semmle/globalVars/UncontrolledFormatStringThroughGlobalVar.expected +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-134/semmle/globalVars/UncontrolledFormatStringThroughGlobalVar.expected @@ -1,5 +1,65 @@ -| globalVars.c:27:9:27:12 | copy | This value may flow through $@, originating from $@, and is a formatting argument to printf(format). | globalVars.c:8:7:8:10 | copy | copy | globalVars.c:24:11:24:14 | argv | argv | -| globalVars.c:30:15:30:18 | copy | This value may flow through $@, originating from $@, and is a formatting argument to printWrapper(str), which calls printf(format). | globalVars.c:8:7:8:10 | copy | copy | globalVars.c:24:11:24:14 | argv | argv | -| globalVars.c:38:9:38:13 | copy2 | This value may flow through $@, originating from $@, and is a formatting argument to printf(format). | globalVars.c:9:7:9:11 | copy2 | copy2 | globalVars.c:24:11:24:14 | argv | argv | -| globalVars.c:41:15:41:19 | copy2 | This value may flow through $@, originating from $@, and is a formatting argument to printWrapper(str), which calls printf(format). | globalVars.c:9:7:9:11 | copy2 | copy2 | globalVars.c:24:11:24:14 | argv | argv | -| globalVars.c:50:9:50:13 | copy2 | This value may flow through $@, originating from $@, and is a formatting argument to printf(format). | globalVars.c:9:7:9:11 | copy2 | copy2 | globalVars.c:24:11:24:14 | argv | argv | +edges +| globalVars.c:8:7:8:10 | copy | globalVars.c:27:9:27:12 | copy | +| globalVars.c:8:7:8:10 | copy | globalVars.c:27:9:27:12 | copy | +| globalVars.c:8:7:8:10 | copy | globalVars.c:27:9:27:12 | copy | +| globalVars.c:8:7:8:10 | copy | globalVars.c:30:15:30:18 | copy | +| globalVars.c:8:7:8:10 | copy | globalVars.c:30:15:30:18 | copy | +| globalVars.c:8:7:8:10 | copy | globalVars.c:35:11:35:14 | copy | +| globalVars.c:9:7:9:11 | copy2 | globalVars.c:38:9:38:13 | copy2 | +| globalVars.c:9:7:9:11 | copy2 | globalVars.c:38:9:38:13 | copy2 | +| globalVars.c:9:7:9:11 | copy2 | globalVars.c:38:9:38:13 | copy2 | +| globalVars.c:9:7:9:11 | copy2 | globalVars.c:41:15:41:19 | copy2 | +| globalVars.c:9:7:9:11 | copy2 | globalVars.c:41:15:41:19 | copy2 | +| globalVars.c:9:7:9:11 | copy2 | globalVars.c:50:9:50:13 | copy2 | +| globalVars.c:9:7:9:11 | copy2 | globalVars.c:50:9:50:13 | copy2 | +| globalVars.c:9:7:9:11 | copy2 | globalVars.c:50:9:50:13 | copy2 | +| globalVars.c:11:22:11:25 | argv | globalVars.c:12:2:12:15 | Store | +| globalVars.c:12:2:12:15 | Store | globalVars.c:8:7:8:10 | copy | +| globalVars.c:15:21:15:23 | val | globalVars.c:16:2:16:12 | Store | +| globalVars.c:16:2:16:12 | Store | globalVars.c:9:7:9:11 | copy2 | +| globalVars.c:24:11:24:14 | argv | globalVars.c:11:22:11:25 | argv | +| globalVars.c:24:11:24:14 | argv | globalVars.c:11:22:11:25 | argv | +| globalVars.c:27:9:27:12 | copy | globalVars.c:27:9:27:12 | (const char *)... | +| globalVars.c:27:9:27:12 | copy | globalVars.c:27:9:27:12 | copy | +| globalVars.c:35:11:35:14 | copy | globalVars.c:15:21:15:23 | val | +| globalVars.c:38:9:38:13 | copy2 | globalVars.c:38:9:38:13 | (const char *)... | +| globalVars.c:38:9:38:13 | copy2 | globalVars.c:38:9:38:13 | copy2 | +| globalVars.c:50:9:50:13 | copy2 | globalVars.c:50:9:50:13 | (const char *)... | +| globalVars.c:50:9:50:13 | copy2 | globalVars.c:50:9:50:13 | copy2 | +nodes +| globalVars.c:8:7:8:10 | copy | semmle.label | copy | +| globalVars.c:9:7:9:11 | copy2 | semmle.label | copy2 | +| globalVars.c:11:22:11:25 | argv | semmle.label | argv | +| globalVars.c:12:2:12:15 | Store | semmle.label | Store | +| globalVars.c:15:21:15:23 | val | semmle.label | val | +| globalVars.c:16:2:16:12 | Store | semmle.label | Store | +| globalVars.c:24:11:24:14 | argv | semmle.label | argv | +| globalVars.c:24:11:24:14 | argv | semmle.label | argv | +| globalVars.c:27:9:27:12 | (const char *)... | semmle.label | (const char *)... | +| globalVars.c:27:9:27:12 | (const char *)... | semmle.label | (const char *)... | +| globalVars.c:27:9:27:12 | copy | semmle.label | copy | +| globalVars.c:27:9:27:12 | copy | semmle.label | copy | +| globalVars.c:27:9:27:12 | copy | semmle.label | copy | +| globalVars.c:30:15:30:18 | copy | semmle.label | copy | +| globalVars.c:30:15:30:18 | copy | semmle.label | copy | +| globalVars.c:30:15:30:18 | copy | semmle.label | copy | +| globalVars.c:35:11:35:14 | copy | semmle.label | copy | +| globalVars.c:38:9:38:13 | (const char *)... | semmle.label | (const char *)... | +| globalVars.c:38:9:38:13 | (const char *)... | semmle.label | (const char *)... | +| globalVars.c:38:9:38:13 | copy2 | semmle.label | copy2 | +| globalVars.c:38:9:38:13 | copy2 | semmle.label | copy2 | +| globalVars.c:38:9:38:13 | copy2 | semmle.label | copy2 | +| globalVars.c:41:15:41:19 | copy2 | semmle.label | copy2 | +| globalVars.c:41:15:41:19 | copy2 | semmle.label | copy2 | +| globalVars.c:41:15:41:19 | copy2 | semmle.label | copy2 | +| globalVars.c:50:9:50:13 | (const char *)... | semmle.label | (const char *)... | +| globalVars.c:50:9:50:13 | (const char *)... | semmle.label | (const char *)... | +| globalVars.c:50:9:50:13 | copy2 | semmle.label | copy2 | +| globalVars.c:50:9:50:13 | copy2 | semmle.label | copy2 | +| globalVars.c:50:9:50:13 | copy2 | semmle.label | copy2 | +#select +| globalVars.c:27:9:27:12 | copy | The value of this argument may come from $@ and is being used as a formatting argument to printf(format) | globalVars.c:24:11:24:14 | argv | argv | +| globalVars.c:30:15:30:18 | copy | The value of this argument may come from $@ and is being used as a formatting argument to printWrapper(str), which calls printf(format) | globalVars.c:24:11:24:14 | argv | argv | +| globalVars.c:38:9:38:13 | copy2 | The value of this argument may come from $@ and is being used as a formatting argument to printf(format) | globalVars.c:24:11:24:14 | argv | argv | +| globalVars.c:41:15:41:19 | copy2 | The value of this argument may come from $@ and is being used as a formatting argument to printWrapper(str), which calls printf(format) | globalVars.c:24:11:24:14 | argv | argv | +| globalVars.c:50:9:50:13 | copy2 | The value of this argument may come from $@ and is being used as a formatting argument to printf(format) | globalVars.c:24:11:24:14 | argv | argv |