Skip to content

Commit b866f1b

Browse files
haby0haby0
committed
Add CWE-348 ClientSuppliedIpUsedInSecurityCheck
1 parent 38f763d commit b866f1b

7 files changed

+295
-0
lines changed

‎python/ql/src/experimental/Security/CWE-348/ClientSuppliedIpUsedInSecurityCheck.py

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
#!/usr/bin/env python
2+
# -*- coding: UTF-8 -*-
3+
"""
4+
@Desc ：ip address spoofing
5+
"""
6+
from flask import Flask
7+
from flask import request
8+
9+
app = Flask(__name__)
10+
11+
@app.route('/bad1')
12+
def bad1():
13+
client_ip = request.headers.get('x-forwarded-for')
14+
if not client_ip.startswith('192.168.'):
15+
raise Exception('ip illegal')
16+
return 'bad1'
17+
18+
@app.route('/bad2')
19+
def bad2():
20+
client_ip = request.headers.get('x-forwarded-for')
21+
if not client_ip == '127.0.0.1':
22+
raise Exception('ip illegal')
23+
return 'bad2'
24+
25+
@app.route('/good1')
26+
def good1():
27+
client_ip = request.headers.get('x-forwarded-for')
28+
client_ip = client_ip.split(',')[client_ip.split(',').length - 1]
29+
if not client_ip == '127.0.0.1':
30+
raise Exception('ip illegal')
31+
return 'good1'
32+
33+
if __name__ == '__main__':
34+
app.debug = True
35+
app.run()

‎python/ql/src/experimental/Security/CWE-348/ClientSuppliedIpUsedInSecurityCheck.qhelp

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>An original client IP address is retrieved from an http header (<code>X-Forwarded-For</code> or <code>X-Real-IP</code> or <code>Proxy-Client-IP</code>
7+
etc.), which is used to ensure security. Attackers can forge the value of these identifiers to
8+
bypass a ban-list, for example.</p>
9+
10+
</overview>
11+
<recommendation>
12+
13+
<p>Do not trust the values of HTTP headers allegedly identifying the originating IP. If you are aware your application will run behind some reverse proxies then the last entry of a <code>X-Forwarded-For</code> header value may be more trustworthy than the rest of it because some reverse proxies append the IP address they observed to the end of any remote-supplied header.</p>
14+
15+
</recommendation>
16+
<example>
17+
18+
<p>The following examples show the bad case and the good case respectively.
19+
In <code>bad1</code> method and <code>bad2</code> method, the client ip the <code>X-Forwarded-For</code> is split into comma-separated values, but the less-trustworthy first one is used. Both of these examples could be deceived by providing a forged HTTP header. The method
20+
<code>good1</code> similarly splits an <code>X-Forwarded-For</code> value, but uses the last, more-trustworthy entry.</p>
21+
22+
<sample src="ClientSuppliedIpUsedInSecurityCheck.py" />
23+
24+
</example>
25+
<references>
26+
27+
<li>Dennis Schneider: <a href="https://www.dennis-schneider.com/blog/prevent-ip-address-spoofing-with-x-forwarded-for-header-and-aws-elb-in-clojure-ring/">
28+
Prevent IP address spoofing with X-Forwarded-For header when using AWS ELB and Clojure Ring</a>
29+
</li>
30+
31+
<li>Security Rule Zero: <a href="https://www.f5.com/company/blog/security-rule-zero-a-warning-about-x-forwarded-for">A Warning about X-Forwarded-For</a>
32+
</li>
33+
34+
</references>
35+
</qhelp>

‎python/ql/src/experimental/Security/CWE-348/ClientSuppliedIpUsedInSecurityCheck.ql

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
/**
2+
* @name IP address spoofing
3+
* @description A remote endpoint identifier is read from an HTTP header. Attackers can modify the value
4+
* of the identifier to forge the client ip.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @precision high
8+
* @id py/ip-address-spoofing
9+
* @tags security
10+
* external/cwe/cwe-348
11+
*/
12+
13+
import python
14+
import semmle.python.dataflow.new.DataFlow
15+
import semmle.python.dataflow.new.TaintTracking
16+
import ClientSuppliedIpUsedInSecurityCheckLib
17+
import DataFlow::PathGraph
18+
19+
/**
20+
* Taint-tracking configuration tracing flow from obtaining a client ip from an HTTP header to a sensitive use.
21+
*/
22+
class ClientSuppliedIpUsedInSecurityCheckConfig extends TaintTracking::Configuration {
23+
ClientSuppliedIpUsedInSecurityCheckConfig() { this = "ClientSuppliedIpUsedInSecurityCheckConfig" }
24+
25+
override predicate isSource(DataFlow::Node source) {
26+
source instanceof ClientSuppliedIpUsedInSecurityCheck
27+
}
28+
29+
override predicate isSink(DataFlow::Node sink) {
30+
sink instanceof ClientSuppliedIpUsedInSecurityCheckSink
31+
}
32+
33+
override predicate isSanitizer(DataFlow::Node node) {
34+
exists(Subscript ss |
35+
not ss.getIndex().(IntegerLiteral).getText() = "0" and
36+
ss.getObject().(Call).getFunc().(Attribute).getName() = "split" and
37+
ss.getObject().(Call).getArg(0).(StrConst).getText() = "," and
38+
ss.getObject().(Call).getFunc().(Attribute).getObject() = node.asExpr()
39+
)
40+
}
41+
}
42+
43+
from
44+
ClientSuppliedIpUsedInSecurityCheckConfig config, DataFlow::PathNode source,
45+
DataFlow::PathNode sink
46+
where config.hasFlowPath(source, sink)
47+
select sink.getNode(), source, sink, "IP address spoofing might include code from $@.",
48+
source.getNode(), "this user input"

‎python/ql/src/experimental/Security/CWE-348/ClientSuppliedIpUsedInSecurityCheckLib.qll

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
private import python
2+
private import semmle.python.Concepts
3+
private import semmle.python.ApiGraphs
4+
private import semmle.python.dataflow.new.DataFlow
5+
private import semmle.python.dataflow.new.RemoteFlowSources
6+
7+
/**
8+
* A data flow source of the client ip obtained according to the remote endpoint identifier specified
9+
* (`X-Forwarded-For`, `X-Real-IP`, `Proxy-Client-IP`, etc.) in the header.
10+
*
11+
* For example: `request.headers.get("X-Forwarded-For")`.
12+
*/
13+
abstract class ClientSuppliedIpUsedInSecurityCheck extends DataFlow::CallCfgNode { }
14+
15+
private class FlaskClientSuppliedIpUsedInSecurityCheck extends ClientSuppliedIpUsedInSecurityCheck {
16+
FlaskClientSuppliedIpUsedInSecurityCheck() {
17+
this =
18+
API::moduleImport("flask")
19+
.getMember("request")
20+
.getMember("headers")
21+
.getMember(["get", "get_all", "getlist"])
22+
.getACall() and
23+
this.getArg(0).asCfgNode().getNode().(StrConst).getText().toLowerCase() =
24+
clientIpParameterName()
25+
}
26+
}
27+
28+
private class DjangoClientSuppliedIpUsedInSecurityCheck extends ClientSuppliedIpUsedInSecurityCheck {
29+
DjangoClientSuppliedIpUsedInSecurityCheck() {
30+
exists(RemoteFlowSource rfs, DataFlow::LocalSourceNode lsn |
31+
rfs.getSourceType() = "django.http.request.HttpRequest" and rfs.asCfgNode() = lsn.asCfgNode()
32+
|
33+
lsn.flowsTo(DataFlow::exprNode(this.getFunction()
34+
.asExpr()
35+
.(Attribute)
36+
.getObject()
37+
.(Attribute)
38+
.getObject())) and
39+
this.getFunction().asExpr().(Attribute).getName() = "get" and
40+
this.getFunction().asExpr().(Attribute).getObject().(Attribute).getName() in [
41+
"headers", "META"
42+
] and
43+
this.getArg(0).asCfgNode().getNode().(StrConst).getText().toLowerCase() =
44+
clientIpParameterName()
45+
)
46+
}
47+
}
48+
49+
private string clientIpParameterName() {
50+
result in [
51+
"x-forwarded-for", "x_forwarded_for", "x-real-ip", "x_real_ip", "proxy-client-ip",
52+
"proxy_client_ip", "wl-proxy-client-ip", "wl_proxy_client_ip", "http_x_forwarded_for",
53+
"http-x-forwarded-for", "http_x_forwarded", "http_x_cluster_client_ip", "http_client_ip",
54+
"http_forwarded_for", "http_forwarded", "http_via", "remote_addr"
55+
]
56+
}
57+
58+
/** A data flow sink for ip address forgery vulnerabilities. */
59+
abstract class ClientSuppliedIpUsedInSecurityCheckSink extends DataFlow::Node { }
60+
61+
/** A data flow sink for sql operation. */
62+
private class SqlOperationSink extends ClientSuppliedIpUsedInSecurityCheckSink {
63+
SqlOperationSink() { this = any(SqlExecution e).getSql() }
64+
}
65+
66+
/**
67+
* A data flow sink for remote client ip comparison.
68+
*
69+
* For example: `if not ipAddr.startswith('192.168.') : ...` determine whether the client ip starts
70+
* with `192.168.`, and the program can be deceived by forging the ip address.
71+
*/
72+
private class CompareSink extends ClientSuppliedIpUsedInSecurityCheckSink {
73+
CompareSink() {
74+
exists(Call call |
75+
call.getFunc().(Attribute).getName() = "startswith" and
76+
call.getArg(0).(StrConst).getText().regexpMatch(getIpAddressRegex()) and
77+
not call.getArg(0).(StrConst).getText() = "0:0:0:0:0:0:0:1" and
78+
call.getFunc().(Attribute).getObject() = this.asExpr()
79+
)
80+
or
81+
exists(Compare compare |
82+
(
83+
compare.getOp(0) instanceof Eq or
84+
compare.getOp(0) instanceof NotEq
85+
) and
86+
(
87+
compare.getLeft() = this.asExpr() and
88+
compare.getComparator(0).(StrConst).getText() instanceof PrivateHostName and
89+
not compare.getComparator(0).(StrConst).getText() = "0:0:0:0:0:0:0:1"
90+
or
91+
compare.getComparator(0) = this.asExpr() and
92+
compare.getLeft().(StrConst).getText() instanceof PrivateHostName and
93+
not compare.getLeft().(StrConst).getText() = "0:0:0:0:0:0:0:1"
94+
)
95+
)
96+
or
97+
exists(Compare compare |
98+
(
99+
compare.getOp(0) instanceof In or
100+
compare.getOp(0) instanceof NotIn
101+
) and
102+
(
103+
compare.getLeft() = this.asExpr()
104+
or
105+
compare.getComparator(0) = this.asExpr()
106+
)
107+
)
108+
or
109+
exists(Call call |
110+
call.getFunc().(Attribute).getName() = "add" and
111+
call.getArg(0) = this.asExpr()
112+
)
113+
}
114+
}
115+
116+
string getIpAddressRegex() {
117+
result =
118+
"^((10\\.((1\\d{2})?|(2[0-4]\\d)?|(25[0-5])?|([1-9]\\d|[0-9])?)(\\.)?)|(192\\.168\\.)|172\\.(1[6789]|2[0-9]|3[01])\\.)((1\\d{2})?|(2[0-4]\\d)?|(25[0-5])?|([1-9]\\d|[0-9])?)(\\.)?((1\\d{2})?|(2[0-4]\\d)?|(25[0-5])?|([1-9]\\d|[0-9])?)$"
119+
}
120+
121+
/**
122+
* A string matching private host names of IPv4 and IPv6, which only matches the host portion therefore checking for port is not necessary.
123+
* Several examples are localhost, reserved IPv4 IP addresses including 127.0.0.1, 10.x.x.x, 172.16.x,x, 192.168.x,x, and reserved IPv6 addresses including [0:0:0:0:0:0:0:1] and [::1]
124+
*/
125+
private class PrivateHostName extends string {
126+
bindingset[this]
127+
PrivateHostName() {
128+
this.regexpMatch("(?i)localhost(?:[:/?#].*)?|127\\.0\\.0\\.1(?:[:/?#].*)?|10(?:\\.[0-9]+){3}(?:[:/?#].*)?|172\\.16(?:\\.[0-9]+){2}(?:[:/?#].*)?|192.168(?:\\.[0-9]+){2}(?:[:/?#].*)?|\\[?0:0:0:0:0:0:0:1\\]?(?:[:/?#].*)?|\\[?::1\\]?(?:[:/?#].*)?")
129+
}
130+
}

‎python/ql/test/experimental/query-tests/Security/CWE-348/ClientSuppliedIpUsedInSecurityCheck.expected

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
edges
2+
| ClientSuppliedIpUsedInSecurityCheck.py:13:17:13:54 | ControlFlowNode for Attribute() | ClientSuppliedIpUsedInSecurityCheck.py:14:12:14:20 | ControlFlowNode for client_ip |
3+
| ClientSuppliedIpUsedInSecurityCheck.py:20:17:20:54 | ControlFlowNode for Attribute() | ClientSuppliedIpUsedInSecurityCheck.py:21:12:21:20 | ControlFlowNode for client_ip |
4+
nodes
5+
| ClientSuppliedIpUsedInSecurityCheck.py:13:17:13:54 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
6+
| ClientSuppliedIpUsedInSecurityCheck.py:14:12:14:20 | ControlFlowNode for client_ip | semmle.label | ControlFlowNode for client_ip |
7+
| ClientSuppliedIpUsedInSecurityCheck.py:20:17:20:54 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
8+
| ClientSuppliedIpUsedInSecurityCheck.py:21:12:21:20 | ControlFlowNode for client_ip | semmle.label | ControlFlowNode for client_ip |
9+
#select
10+
| ClientSuppliedIpUsedInSecurityCheck.py:14:12:14:20 | ControlFlowNode for client_ip | ClientSuppliedIpUsedInSecurityCheck.py:13:17:13:54 | ControlFlowNode for Attribute() | ClientSuppliedIpUsedInSecurityCheck.py:14:12:14:20 | ControlFlowNode for client_ip | IP address spoofing might include code from $@. | ClientSuppliedIpUsedInSecurityCheck.py:13:17:13:54 | ControlFlowNode for Attribute() | this user input |
11+
| ClientSuppliedIpUsedInSecurityCheck.py:21:12:21:20 | ControlFlowNode for client_ip | ClientSuppliedIpUsedInSecurityCheck.py:20:17:20:54 | ControlFlowNode for Attribute() | ClientSuppliedIpUsedInSecurityCheck.py:21:12:21:20 | ControlFlowNode for client_ip | IP address spoofing might include code from $@. | ClientSuppliedIpUsedInSecurityCheck.py:20:17:20:54 | ControlFlowNode for Attribute() | this user input |

‎python/ql/test/experimental/query-tests/Security/CWE-348/ClientSuppliedIpUsedInSecurityCheck.py

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
#!/usr/bin/env python
2+
# -*- coding: UTF-8 -*-
3+
"""
4+
@Desc ：ip address spoofing
5+
"""
6+
from flask import Flask
7+
from flask import request
8+
9+
app = Flask(__name__)
10+
11+
@app.route('/bad1')
12+
def bad1():
13+
client_ip = request.headers.get('x-forwarded-for')
14+
if not client_ip.startswith('192.168.'):
15+
raise Exception('ip illegal')
16+
return 'bad1'
17+
18+
@app.route('/bad2')
19+
def bad2():
20+
client_ip = request.headers.get('x-forwarded-for')
21+
if not client_ip == '127.0.0.1':
22+
raise Exception('ip illegal')
23+
return 'bad2'
24+
25+
@app.route('/good1')
26+
def good1():
27+
client_ip = request.headers.get('x-forwarded-for')
28+
client_ip = client_ip.split(',')[client_ip.split(',').length - 1]
29+
if not client_ip == '127.0.0.1':
30+
raise Exception('ip illegal')
31+
return 'good1'
32+
33+
if __name__ == '__main__':
34+
app.debug = True
35+
app.run()

‎python/ql/test/experimental/query-tests/Security/CWE-348/ClientSuppliedIpUsedInSecurityCheck.qlref

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
experimental/Security/CWE-348/ClientSuppliedIpUsedInSecurityCheck.ql

0 commit comments

Comments
 (0)