Skip to content

Commit 0ccf397

Browse files
rdmarsh2rdmarsh2
authored
Merge pull request github#3189 from jbj/DefaultTaintTracking-Configuration
C++: Path explanations in DefaultTaintTracking
2 parents 8928091 + 057155f commit 0ccf397

File tree

31 files changed

+1469
-197
lines changed

31 files changed

+1469
-197
lines changed

‎cpp/ql/src/Security/CWE/CWE-022/TaintedPath.ql

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
* @name Uncontrolled data used in path expression
33
* @description Accessing paths influenced by users can allow an
44
* attacker to access unexpected resources.
5-
* @kind problem
5+
* @kind path-problem
66
* @problem.severity warning
77
* @precision medium
88
* @id cpp/path-injection
@@ -17,6 +17,7 @@ import cpp
1717
import semmle.code.cpp.security.FunctionWithWrappers
1818
import semmle.code.cpp.security.Security
1919
import semmle.code.cpp.security.TaintTracking
20+
import TaintedWithPath
2021

2122
/**
2223
* A function for opening a file.
@@ -51,12 +52,19 @@ class FileFunction extends FunctionWithWrappers {
5152
override predicate interestingArg(int arg) { arg = 0 }
5253
}
5354

55+
class TaintedPathConfiguration extends TaintTrackingConfiguration {
56+
override predicate isSink(Element tainted) {
57+
exists(FileFunction fileFunction | fileFunction.outermostWrapperFunctionCall(tainted, _))
58+
}
59+
}
60+
5461
from
55-
FileFunction fileFunction, Expr taintedArg, Expr taintSource, string taintCause, string callChain
62+
FileFunction fileFunction, Expr taintedArg, Expr taintSource, PathNode sourceNode,
63+
PathNode sinkNode, string taintCause, string callChain
5664
where
5765
fileFunction.outermostWrapperFunctionCall(taintedArg, callChain) and
58-
tainted(taintSource, taintedArg) and
66+
taintedWithPath(taintSource, taintedArg, sourceNode, sinkNode) and
5967
isUserInput(taintSource, taintCause)
60-
select taintedArg,
68+
select taintedArg, sourceNode, sinkNode,
6169
"This argument to a file access function is derived from $@ and then passed to " + callChain,
6270
taintSource, "user input (" + taintCause + ")"

‎cpp/ql/src/Security/CWE/CWE-079/CgiXss.ql

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
* @name CGI script vulnerable to cross-site scripting
33
* @description Writing user input directly to a web page
44
* allows for a cross-site scripting vulnerability.
5-
* @kind problem
5+
* @kind path-problem
66
* @problem.severity error
77
* @precision high
88
* @id cpp/cgi-xss
@@ -13,6 +13,7 @@
1313
import cpp
1414
import semmle.code.cpp.commons.Environment
1515
import semmle.code.cpp.security.TaintTracking
16+
import TaintedWithPath
1617

1718
/** A call that prints its arguments to `stdout`. */
1819
class PrintStdoutCall extends FunctionCall {
@@ -27,8 +28,13 @@ class QueryString extends EnvironmentRead {
2728
QueryString() { getEnvironmentVariable() = "QUERY_STRING" }
2829
}
2930

30-
from QueryString query, PrintStdoutCall call, Element printedArg
31-
where
32-
call.getAnArgument() = printedArg and
33-
tainted(query, printedArg)
34-
select printedArg, "Cross-site scripting vulnerability due to $@.", query, "this query data"
31+
class Configuration extends TaintTrackingConfiguration {
32+
override predicate isSink(Element tainted) {
33+
exists(PrintStdoutCall call | call.getAnArgument() = tainted)
34+
}
35+
}
36+
37+
from QueryString query, Element printedArg, PathNode sourceNode, PathNode sinkNode
38+
where taintedWithPath(query, printedArg, sourceNode, sinkNode)
39+
select printedArg, sourceNode, sinkNode, "Cross-site scripting vulnerability due to $@.", query,
40+
"this query data"

‎cpp/ql/src/Security/CWE/CWE-089/SqlTainted.ql

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* @description Including user-supplied data in a SQL query without
44
* neutralizing special elements can make code vulnerable
55
* to SQL Injection.
6-
* @kind problem
6+
* @kind path-problem
77
* @problem.severity error
88
* @precision high
99
* @id cpp/sql-injection
@@ -15,18 +15,27 @@ import cpp
1515
import semmle.code.cpp.security.Security
1616
import semmle.code.cpp.security.FunctionWithWrappers
1717
import semmle.code.cpp.security.TaintTracking
18+
import TaintedWithPath
1819

1920
class SQLLikeFunction extends FunctionWithWrappers {
2021
SQLLikeFunction() { sqlArgument(this.getName(), _) }
2122

2223
override predicate interestingArg(int arg) { sqlArgument(this.getName(), arg) }
2324
}
2425

25-
from SQLLikeFunction runSql, Expr taintedArg, Expr taintSource, string taintCause, string callChain
26+
class Configuration extends TaintTrackingConfiguration {
27+
override predicate isSink(Element tainted) {
28+
exists(SQLLikeFunction runSql | runSql.outermostWrapperFunctionCall(tainted, _))
29+
}
30+
}
31+
32+
from
33+
SQLLikeFunction runSql, Expr taintedArg, Expr taintSource, PathNode sourceNode, PathNode sinkNode,
34+
string taintCause, string callChain
2635
where
2736
runSql.outermostWrapperFunctionCall(taintedArg, callChain) and
28-
tainted(taintSource, taintedArg) and
37+
taintedWithPath(taintSource, taintedArg, sourceNode, sinkNode) and
2938
isUserInput(taintSource, taintCause)
30-
select taintedArg,
39+
select taintedArg, sourceNode, sinkNode,
3140
"This argument to a SQL query function is derived from $@ and then passed to " + callChain,
3241
taintSource, "user input (" + taintCause + ")"

‎cpp/ql/src/Security/CWE/CWE-114/UncontrolledProcessOperation.ql

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* @description Using externally controlled strings in a process
44
* operation can allow an attacker to execute malicious
55
* commands.
6-
* @kind problem
6+
* @kind path-problem
77
* @problem.severity warning
88
* @precision medium
99
* @id cpp/uncontrolled-process-operation
@@ -14,13 +14,24 @@
1414
import cpp
1515
import semmle.code.cpp.security.Security
1616
import semmle.code.cpp.security.TaintTracking
17+
import TaintedWithPath
1718

18-
from string processOperation, int processOperationArg, FunctionCall call, Expr arg, Element source
19+
predicate isProcessOperationExplanation(Expr arg, string processOperation) {
20+
exists(int processOperationArg, FunctionCall call |
21+
isProcessOperationArgument(processOperation, processOperationArg) and
22+
call.getTarget().getName() = processOperation and
23+
call.getArgument(processOperationArg) = arg
24+
)
25+
}
26+
27+
class Configuration extends TaintTrackingConfiguration {
28+
override predicate isSink(Element arg) { isProcessOperationExplanation(arg, _) }
29+
}
30+
31+
from string processOperation, Expr arg, Expr source, PathNode sourceNode, PathNode sinkNode
1932
where
20-
isProcessOperationArgument(processOperation, processOperationArg) and
21-
call.getTarget().getName() = processOperation and
22-
call.getArgument(processOperationArg) = arg and
23-
tainted(source, arg)
24-
select arg,
33+
isProcessOperationExplanation(arg, processOperation) and
34+
taintedWithPath(source, arg, sourceNode, sinkNode)
35+
select arg, sourceNode, sinkNode,
2536
"The value of this argument may come from $@ and is being passed to " + processOperation, source,
2637
source.toString()

‎cpp/ql/src/Security/CWE/CWE-120/UnboundedWrite.ql

Lines changed: 36 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
* @name Unbounded write
33
* @description Buffer write operations that do not control the length
44
* of data written may overflow.
5-
* @kind problem
5+
* @kind path-problem
66
* @problem.severity error
77
* @precision medium
88
* @id cpp/unbounded-write
@@ -16,6 +16,7 @@
1616
import semmle.code.cpp.security.BufferWrite
1717
import semmle.code.cpp.security.Security
1818
import semmle.code.cpp.security.TaintTracking
19+
import TaintedWithPath
1920

2021
/*
2122
* --- Summary of CWE-120 alerts ---
@@ -54,32 +55,48 @@ predicate isUnboundedWrite(BufferWrite bw) {
5455
* }
5556
*/
5657

58+
/**
59+
* Holds if `e` is a source buffer going into an unbounded write `bw` or a
60+
* qualifier of (a qualifier of ...) such a source.
61+
*/
62+
predicate unboundedWriteSource(Expr e, BufferWrite bw) {
63+
isUnboundedWrite(bw) and e = bw.getASource()
64+
or
65+
exists(FieldAccess fa | unboundedWriteSource(fa, bw) and e = fa.getQualifier())
66+
}
67+
5768
/*
5869
* --- user input reach ---
5970
*/
6071

61-
/**
62-
* Identifies expressions that are potentially tainted with user
63-
* input. Most of the work for this is actually done by the
64-
* TaintTracking library.
65-
*/
66-
predicate tainted2(Expr expr, Expr inputSource, string inputCause) {
67-
taintedIncludingGlobalVars(inputSource, expr, _) and
68-
inputCause = inputSource.toString()
69-
or
70-
exists(Expr e | tainted2(e, inputSource, inputCause) |
71-
// field accesses of a tainted struct are tainted
72-
e = expr.(FieldAccess).getQualifier()
73-
)
72+
class Configuration extends TaintTrackingConfiguration {
73+
override predicate isSink(Element tainted) { unboundedWriteSource(tainted, _) }
74+
75+
override predicate taintThroughGlobals() { any() }
7476
}
7577

7678
/*
7779
* --- put it together ---
7880
*/
7981

80-
from BufferWrite bw, Expr inputSource, string inputCause
82+
/*
83+
* An unbounded write is, for example `strcpy(..., tainted)`. We're looking
84+
* for a tainted source buffer of an unbounded write, where this source buffer
85+
* is a sink in the taint-tracking analysis.
86+
*
87+
* In the case of `gets` and `scanf`, where the source buffer is implicit, the
88+
* `BufferWrite` library reports the source buffer to be the same as the
89+
* destination buffer. Since those destination-buffer arguments are also
90+
* modeled in the taint-tracking library as being _sources_ of taint, they are
91+
* in practice reported as being tainted because the `security.TaintTracking`
92+
* library does not distinguish between taint going into an argument and out of
93+
* an argument. Thus, we get the desired alerts.
94+
*/
95+
96+
from BufferWrite bw, Expr inputSource, Expr tainted, PathNode sourceNode, PathNode sinkNode
8197
where
82-
isUnboundedWrite(bw) and
83-
tainted2(bw.getASource(), inputSource, inputCause)
84-
select bw, "This '" + bw.getBWDesc() + "' with input from $@ may overflow the destination.",
85-
inputSource, inputCause
98+
taintedWithPath(inputSource, tainted, sourceNode, sinkNode) and
99+
unboundedWriteSource(tainted, bw)
100+
select bw, sourceNode, sinkNode,
101+
"This '" + bw.getBWDesc() + "' with input from $@ may overflow the destination.", inputSource,
102+
inputSource.toString()

‎cpp/ql/src/Security/CWE/CWE-134/UncontrolledFormatString.ql

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* @description Using externally-controlled format strings in
44
* printf-style functions can lead to buffer overflows
55
* or data representation problems.
6-
* @kind problem
6+
* @kind path-problem
77
* @problem.severity warning
88
* @precision medium
99
* @id cpp/tainted-format-string
@@ -16,12 +16,21 @@ import cpp
1616
import semmle.code.cpp.security.Security
1717
import semmle.code.cpp.security.FunctionWithWrappers
1818
import semmle.code.cpp.security.TaintTracking
19+
import TaintedWithPath
1920

20-
from PrintfLikeFunction printf, Expr arg, string printfFunction, Expr userValue, string cause
21+
class Configuration extends TaintTrackingConfiguration {
22+
override predicate isSink(Element tainted) {
23+
exists(PrintfLikeFunction printf | printf.outermostWrapperFunctionCall(tainted, _))
24+
}
25+
}
26+
27+
from
28+
PrintfLikeFunction printf, Expr arg, PathNode sourceNode, PathNode sinkNode,
29+
string printfFunction, Expr userValue, string cause
2130
where
2231
printf.outermostWrapperFunctionCall(arg, printfFunction) and
23-
tainted(userValue, arg) and
32+
taintedWithPath(userValue, arg, sourceNode, sinkNode) and
2433
isUserInput(userValue, cause)
25-
select arg,
34+
select arg, sourceNode, sinkNode,
2635
"The value of this argument may come from $@ and is being used as a formatting argument to " +
2736
printfFunction, userValue, cause

‎cpp/ql/src/Security/CWE/CWE-134/UncontrolledFormatStringThroughGlobalVar.ql

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* @description Using externally-controlled format strings in
44
* printf-style functions can lead to buffer overflows
55
* or data representation problems.
6-
* @kind problem
6+
* @kind path-problem
77
* @problem.severity warning
88
* @precision medium
99
* @id cpp/tainted-format-string-through-global
@@ -16,15 +16,24 @@ import cpp
1616
import semmle.code.cpp.security.FunctionWithWrappers
1717
import semmle.code.cpp.security.Security
1818
import semmle.code.cpp.security.TaintTracking
19+
import TaintedWithPath
20+
21+
class Configuration extends TaintTrackingConfiguration {
22+
override predicate isSink(Element tainted) {
23+
exists(PrintfLikeFunction printf | printf.outermostWrapperFunctionCall(tainted, _))
24+
}
25+
26+
override predicate taintThroughGlobals() { any() }
27+
}
1928

2029
from
21-
PrintfLikeFunction printf, Expr arg, string printfFunction, Expr userValue, string cause,
22-
string globalVar
30+
PrintfLikeFunction printf, Expr arg, PathNode sourceNode, PathNode sinkNode,
31+
string printfFunction, Expr userValue, string cause
2332
where
2433
printf.outermostWrapperFunctionCall(arg, printfFunction) and
25-
not tainted(_, arg) and
26-
taintedIncludingGlobalVars(userValue, arg, globalVar) and
34+
not taintedWithoutGlobals(arg) and
35+
taintedWithPath(userValue, arg, sourceNode, sinkNode) and
2736
isUserInput(userValue, cause)
28-
select arg,
29-
"This value may flow through $@, originating from $@, and is a formatting argument to " +
30-
printfFunction + ".", globalVarFromId(globalVar), globalVar, userValue, cause
37+
select arg, sourceNode, sinkNode,
38+
"The value of this argument may come from $@ and is being used as a formatting argument to " +
39+
printfFunction, userValue, cause

‎cpp/ql/src/Security/CWE/CWE-190/ArithmeticUncontrolled.ql

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
* @name Uncontrolled data in arithmetic expression
33
* @description Arithmetic operations on uncontrolled data that is not
44
* validated can cause overflows.
5-
* @kind problem
5+
* @kind path-problem
66
* @problem.severity warning
77
* @precision medium
88
* @id cpp/uncontrolled-arithmetic
@@ -15,6 +15,7 @@ import cpp
1515
import semmle.code.cpp.security.Overflow
1616
import semmle.code.cpp.security.Security
1717
import semmle.code.cpp.security.TaintTracking
18+
import TaintedWithPath
1819

1920
predicate isRandCall(FunctionCall fc) { fc.getTarget().getName() = "rand" }
2021

@@ -40,29 +41,40 @@ class SecurityOptionsArith extends SecurityOptions {
4041
}
4142
}
4243

43-
predicate taintedVarAccess(Expr origin, VariableAccess va) {
44-
isUserInput(origin, _) and
45-
tainted(origin, va)
44+
predicate isDiv(VariableAccess va) { exists(AssignDivExpr div | div.getLValue() = va) }
45+
46+
predicate missingGuard(VariableAccess va, string effect) {
47+
exists(Operation op | op.getAnOperand() = va |
48+
missingGuardAgainstUnderflow(op, va) and effect = "underflow"
49+
or
50+
missingGuardAgainstOverflow(op, va) and effect = "overflow"
51+
)
52+
}
53+
54+
class Configuration extends TaintTrackingConfiguration {
55+
override predicate isSink(Element e) {
56+
isDiv(e)
57+
or
58+
missingGuard(e, _)
59+
}
4660
}
4761

4862
/**
4963
* A value that undergoes division is likely to be bounded within a safe
5064
* range.
5165
*/
5266
predicate guardedByAssignDiv(Expr origin) {
53-
isUserInput(origin, _) and
54-
exists(AssignDivExpr div, VariableAccess va | tainted(origin, va) and div.getLValue() = va)
67+
exists(VariableAccess va |
68+
taintedWithPath(origin, va, _, _) and
69+
isDiv(va)
70+
)
5571
}
5672

57-
from Expr origin, Operation op, VariableAccess va, string effect
73+
from Expr origin, VariableAccess va, string effect, PathNode sourceNode, PathNode sinkNode
5874
where
59-
taintedVarAccess(origin, va) and
60-
op.getAnOperand() = va and
61-
(
62-
missingGuardAgainstUnderflow(op, va) and effect = "underflow"
63-
or
64-
missingGuardAgainstOverflow(op, va) and effect = "overflow"
65-
) and
75+
taintedWithPath(origin, va, sourceNode, sinkNode) and
76+
missingGuard(va, effect) and
6677
not guardedByAssignDiv(origin)
67-
select va, "$@ flows to here and is used in arithmetic, potentially causing an " + effect + ".",
68-
origin, "Uncontrolled value"
78+
select va, sourceNode, sinkNode,
79+
"$@ flows to here and is used in arithmetic, potentially causing an " + effect + ".", origin,
80+
"Uncontrolled value"

0 commit comments

Comments
 (0)