Add CWE-348 ClientSuppliedIpUsedInSecurityCheck

This commit is contained in:
haby0
2021-07-02 19:30:33 +08:00
parent 38f763dd6a
commit b866f1b21e
7 changed files with 295 additions and 0 deletions

View File

@@ -0,0 +1,35 @@
#!/usr/bin/env python
# -*- coding: UTF-8 -*-
"""
@Desc ip address spoofing
"""
from flask import Flask
from flask import request
app = Flask(__name__)
@app.route('/bad1')
def bad1():
client_ip = request.headers.get('x-forwarded-for')
if not client_ip.startswith('192.168.'):
raise Exception('ip illegal')
return 'bad1'
@app.route('/bad2')
def bad2():
client_ip = request.headers.get('x-forwarded-for')
if not client_ip == '127.0.0.1':
raise Exception('ip illegal')
return 'bad2'
@app.route('/good1')
def good1():
client_ip = request.headers.get('x-forwarded-for')
client_ip = client_ip.split(',')[client_ip.split(',').length - 1]
if not client_ip == '127.0.0.1':
raise Exception('ip illegal')
return 'good1'
if __name__ == '__main__':
app.debug = True
app.run()

View File

@@ -0,0 +1,35 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<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>
etc.), which is used to ensure security. Attackers can forge the value of these identifiers to
bypass a ban-list, for example.</p>
</overview>
<recommendation>
<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>
</recommendation>
<example>
<p>The following examples show the bad case and the good case respectively.
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
<code>good1</code> similarly splits an <code>X-Forwarded-For</code> value, but uses the last, more-trustworthy entry.</p>
<sample src="ClientSuppliedIpUsedInSecurityCheck.py" />
</example>
<references>
<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/">
Prevent IP address spoofing with X-Forwarded-For header when using AWS ELB and Clojure Ring</a>
</li>
<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>
</li>
</references>
</qhelp>

View File

@@ -0,0 +1,48 @@
/**
* @name IP address spoofing
* @description A remote endpoint identifier is read from an HTTP header. Attackers can modify the value
* of the identifier to forge the client ip.
* @kind path-problem
* @problem.severity error
* @precision high
* @id py/ip-address-spoofing
* @tags security
* external/cwe/cwe-348
*/
import python
import semmle.python.dataflow.new.DataFlow
import semmle.python.dataflow.new.TaintTracking
import ClientSuppliedIpUsedInSecurityCheckLib
import DataFlow::PathGraph
/**
* Taint-tracking configuration tracing flow from obtaining a client ip from an HTTP header to a sensitive use.
*/
class ClientSuppliedIpUsedInSecurityCheckConfig extends TaintTracking::Configuration {
ClientSuppliedIpUsedInSecurityCheckConfig() { this = "ClientSuppliedIpUsedInSecurityCheckConfig" }
override predicate isSource(DataFlow::Node source) {
source instanceof ClientSuppliedIpUsedInSecurityCheck
}
override predicate isSink(DataFlow::Node sink) {
sink instanceof ClientSuppliedIpUsedInSecurityCheckSink
}
override predicate isSanitizer(DataFlow::Node node) {
exists(Subscript ss |
not ss.getIndex().(IntegerLiteral).getText() = "0" and
ss.getObject().(Call).getFunc().(Attribute).getName() = "split" and
ss.getObject().(Call).getArg(0).(StrConst).getText() = "," and
ss.getObject().(Call).getFunc().(Attribute).getObject() = node.asExpr()
)
}
}
from
ClientSuppliedIpUsedInSecurityCheckConfig config, DataFlow::PathNode source,
DataFlow::PathNode sink
where config.hasFlowPath(source, sink)
select sink.getNode(), source, sink, "IP address spoofing might include code from $@.",
source.getNode(), "this user input"

View File

@@ -0,0 +1,130 @@
private import python
private import semmle.python.Concepts
private import semmle.python.ApiGraphs
private import semmle.python.dataflow.new.DataFlow
private import semmle.python.dataflow.new.RemoteFlowSources
/**
* A data flow source of the client ip obtained according to the remote endpoint identifier specified
* (`X-Forwarded-For`, `X-Real-IP`, `Proxy-Client-IP`, etc.) in the header.
*
* For example: `request.headers.get("X-Forwarded-For")`.
*/
abstract class ClientSuppliedIpUsedInSecurityCheck extends DataFlow::CallCfgNode { }
private class FlaskClientSuppliedIpUsedInSecurityCheck extends ClientSuppliedIpUsedInSecurityCheck {
FlaskClientSuppliedIpUsedInSecurityCheck() {
this =
API::moduleImport("flask")
.getMember("request")
.getMember("headers")
.getMember(["get", "get_all", "getlist"])
.getACall() and
this.getArg(0).asCfgNode().getNode().(StrConst).getText().toLowerCase() =
clientIpParameterName()
}
}
private class DjangoClientSuppliedIpUsedInSecurityCheck extends ClientSuppliedIpUsedInSecurityCheck {
DjangoClientSuppliedIpUsedInSecurityCheck() {
exists(RemoteFlowSource rfs, DataFlow::LocalSourceNode lsn |
rfs.getSourceType() = "django.http.request.HttpRequest" and rfs.asCfgNode() = lsn.asCfgNode()
|
lsn.flowsTo(DataFlow::exprNode(this.getFunction()
.asExpr()
.(Attribute)
.getObject()
.(Attribute)
.getObject())) and
this.getFunction().asExpr().(Attribute).getName() = "get" and
this.getFunction().asExpr().(Attribute).getObject().(Attribute).getName() in [
"headers", "META"
] and
this.getArg(0).asCfgNode().getNode().(StrConst).getText().toLowerCase() =
clientIpParameterName()
)
}
}
private string clientIpParameterName() {
result in [
"x-forwarded-for", "x_forwarded_for", "x-real-ip", "x_real_ip", "proxy-client-ip",
"proxy_client_ip", "wl-proxy-client-ip", "wl_proxy_client_ip", "http_x_forwarded_for",
"http-x-forwarded-for", "http_x_forwarded", "http_x_cluster_client_ip", "http_client_ip",
"http_forwarded_for", "http_forwarded", "http_via", "remote_addr"
]
}
/** A data flow sink for ip address forgery vulnerabilities. */
abstract class ClientSuppliedIpUsedInSecurityCheckSink extends DataFlow::Node { }
/** A data flow sink for sql operation. */
private class SqlOperationSink extends ClientSuppliedIpUsedInSecurityCheckSink {
SqlOperationSink() { this = any(SqlExecution e).getSql() }
}
/**
* A data flow sink for remote client ip comparison.
*
* For example: `if not ipAddr.startswith('192.168.') : ...` determine whether the client ip starts
* with `192.168.`, and the program can be deceived by forging the ip address.
*/
private class CompareSink extends ClientSuppliedIpUsedInSecurityCheckSink {
CompareSink() {
exists(Call call |
call.getFunc().(Attribute).getName() = "startswith" and
call.getArg(0).(StrConst).getText().regexpMatch(getIpAddressRegex()) and
not call.getArg(0).(StrConst).getText() = "0:0:0:0:0:0:0:1" and
call.getFunc().(Attribute).getObject() = this.asExpr()
)
or
exists(Compare compare |
(
compare.getOp(0) instanceof Eq or
compare.getOp(0) instanceof NotEq
) and
(
compare.getLeft() = this.asExpr() and
compare.getComparator(0).(StrConst).getText() instanceof PrivateHostName and
not compare.getComparator(0).(StrConst).getText() = "0:0:0:0:0:0:0:1"
or
compare.getComparator(0) = this.asExpr() and
compare.getLeft().(StrConst).getText() instanceof PrivateHostName and
not compare.getLeft().(StrConst).getText() = "0:0:0:0:0:0:0:1"
)
)
or
exists(Compare compare |
(
compare.getOp(0) instanceof In or
compare.getOp(0) instanceof NotIn
) and
(
compare.getLeft() = this.asExpr()
or
compare.getComparator(0) = this.asExpr()
)
)
or
exists(Call call |
call.getFunc().(Attribute).getName() = "add" and
call.getArg(0) = this.asExpr()
)
}
}
string getIpAddressRegex() {
result =
"^((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])?)$"
}
/**
* A string matching private host names of IPv4 and IPv6, which only matches the host portion therefore checking for port is not necessary.
* 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]
*/
private class PrivateHostName extends string {
bindingset[this]
PrivateHostName() {
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\\]?(?:[:/?#].*)?")
}
}