Skip to content

C++: Rewrite cpp/cgi-xss to not use default taint tracking #11716

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jketema merged 1 commit into from
Oct 9, 2023
Merged

C++: Rewrite cpp/cgi-xss to not use default taint tracking #11716

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 18 additions & 16 deletions cpp/ql/src/Security/CWE/CWE-079/CgiXss.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,35 +13,37 @@

import cpp
import semmle.code.cpp.commons.Environment
import semmle.code.cpp.ir.dataflow.internal.DefaultTaintTrackingImpl
import TaintedWithPath
import semmle.code.cpp.ir.dataflow.TaintTracking
import semmle.code.cpp.ir.IR
import Flow::PathGraph

/** A call that prints its arguments to `stdout`. */
class PrintStdoutCall extends FunctionCall {
PrintStdoutCall() {
this.getTarget().hasGlobalOrStdName("puts") or
this.getTarget().hasGlobalOrStdName("printf")
}
PrintStdoutCall() { this.getTarget().hasGlobalOrStdName(["puts", "printf"]) }
}

/** A read of the QUERY_STRING environment variable */
class QueryString extends EnvironmentRead {
QueryString() { this.getEnvironmentVariable() = "QUERY_STRING" }
}

class Configuration extends TaintTrackingConfiguration {
override predicate isSource(Expr source) { source instanceof QueryString }
module Config implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node node) { node.asExpr() instanceof QueryString }

override predicate isSink(Element tainted) {
exists(PrintStdoutCall call | call.getAnArgument() = tainted)
predicate isSink(DataFlow::Node node) {
exists(PrintStdoutCall call | call.getAnArgument() = node.asExpr())
}

override predicate isBarrier(Expr e) {
super.isBarrier(e) or e.getUnspecifiedType() instanceof IntegralType
predicate isBarrier(DataFlow::Node node) {
node.asExpr().getUnspecifiedType() instanceof IntegralType
}
}

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"
module Flow = TaintTracking::Global<Config>;

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"
9 changes: 1 addition & 8 deletions cpp/ql/test/query-tests/Security/CWE/CWE-079/semmle/CgiXss/CgiXss.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,26 +1,19 @@
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 |