From 6ff8e06ace18cdc6c7fc544aa1fd8f8d00bc8cd4 Mon Sep 17 00:00:00 2001 From: Jeroen Ketema Date: Mon, 9 Oct 2023 16:30:21 +0200 Subject: [PATCH] Revert "C++: Rewrite `cpp/cgi-xss` to not use default taint tracking" This reverts commit b6132d2a0fcd7496d0e9145a9519e567e2e2f665. --- cpp/ql/src/Security/CWE/CWE-079/CgiXss.ql | 34 +++++++++---------- .../CWE/CWE-079/semmle/CgiXss/CgiXss.expected | 9 ++++- 2 files changed, 24 insertions(+), 19 deletions(-) diff --git a/cpp/ql/src/Security/CWE/CWE-079/CgiXss.ql b/cpp/ql/src/Security/CWE/CWE-079/CgiXss.ql index b24147a12b6..e16f0568056 100644 --- a/cpp/ql/src/Security/CWE/CWE-079/CgiXss.ql +++ b/cpp/ql/src/Security/CWE/CWE-079/CgiXss.ql @@ -13,13 +13,15 @@ import cpp import semmle.code.cpp.commons.Environment -import semmle.code.cpp.ir.dataflow.TaintTracking -import semmle.code.cpp.ir.IR -import Flow::PathGraph +import semmle.code.cpp.ir.dataflow.internal.DefaultTaintTrackingImpl +import TaintedWithPath /** A call that prints its arguments to `stdout`. */ class PrintStdoutCall extends FunctionCall { - PrintStdoutCall() { this.getTarget().hasGlobalOrStdName(["puts", "printf"]) } + PrintStdoutCall() { + this.getTarget().hasGlobalOrStdName("puts") or + this.getTarget().hasGlobalOrStdName("printf") + } } /** A read of the QUERY_STRING environment variable */ @@ -27,23 +29,19 @@ class QueryString extends EnvironmentRead { QueryString() { this.getEnvironmentVariable() = "QUERY_STRING" } } -module Config implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node node) { node.asExpr() instanceof QueryString } +class Configuration extends TaintTrackingConfiguration { + override predicate isSource(Expr source) { source instanceof QueryString } - predicate isSink(DataFlow::Node node) { - exists(PrintStdoutCall call | call.getAnArgument() = node.asExpr()) + override predicate isSink(Element tainted) { + exists(PrintStdoutCall call | call.getAnArgument() = tainted) } - predicate isBarrier(DataFlow::Node node) { - node.asExpr().getUnspecifiedType() instanceof IntegralType + override predicate isBarrier(Expr e) { + super.isBarrier(e) or e.getUnspecifiedType() instanceof IntegralType } } -module Flow = TaintTracking::Global; - -from QueryString query, Flow::PathNode sourceNode, Flow::PathNode sinkNode -where - Flow::flowPath(sourceNode, sinkNode) and - query = sourceNode.getNode().asExpr() -select sinkNode.getNode(), sourceNode, sinkNode, "Cross-site scripting vulnerability due to $@.", - query, "this query data" +from QueryString query, Element printedArg, PathNode sourceNode, PathNode sinkNode +where taintedWithPath(query, printedArg, sourceNode, sinkNode) +select printedArg, sourceNode, sinkNode, "Cross-site scripting vulnerability due to $@.", query, + "this query data" diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-079/semmle/CgiXss/CgiXss.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-079/semmle/CgiXss/CgiXss.expected index 3d46e42dfd9..52885ef1df9 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-079/semmle/CgiXss/CgiXss.expected +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-079/semmle/CgiXss/CgiXss.expected @@ -1,19 +1,26 @@ edges | search.c:14:24:14:28 | query | search.c:17:8:17:12 | query | +| search.c:14:24:14:28 | query | search.c:17:8:17:12 | query | +| search.c:22:24:22:28 | query | search.c:23:39:23:43 | query | | search.c:22:24:22:28 | query | search.c:23:39:23:43 | query | | search.c:51:21:51:26 | call to getenv | search.c:55:17:55:25 | raw_query | +| search.c:51:21:51:26 | call to getenv | search.c:55:17:55:25 | raw_query | +| search.c:51:21:51:26 | call to getenv | search.c:57:17:57:25 | raw_query | | search.c:51:21:51:26 | call to getenv | search.c:57:17:57:25 | raw_query | | search.c:55:17:55:25 | raw_query | search.c:14:24:14:28 | query | | search.c:57:17:57:25 | raw_query | search.c:22:24:22:28 | query | +subpaths nodes | search.c:14:24:14:28 | query | semmle.label | query | | search.c:17:8:17:12 | query | semmle.label | query | +| search.c:17:8:17:12 | query | semmle.label | query | | search.c:22:24:22:28 | query | semmle.label | query | | search.c:23:39:23:43 | query | semmle.label | query | +| search.c:23:39:23:43 | query | semmle.label | query | +| search.c:51:21:51:26 | call to getenv | semmle.label | call to getenv | | search.c:51:21:51:26 | call to getenv | semmle.label | call to getenv | | search.c:55:17:55:25 | raw_query | semmle.label | raw_query | | search.c:57:17:57:25 | raw_query | semmle.label | raw_query | -subpaths #select | search.c:17:8:17:12 | query | search.c:51:21:51:26 | call to getenv | search.c:17:8:17:12 | query | Cross-site scripting vulnerability due to $@. | search.c:51:21:51:26 | call to getenv | this query data | | search.c:23:39:23:43 | query | search.c:51:21:51:26 | call to getenv | search.c:23:39:23:43 | query | Cross-site scripting vulnerability due to $@. | search.c:51:21:51:26 | call to getenv | this query data |