Skip to content

Commit 5054d5b

Browse files
codeql-cicodeql-ci
authored
Merge pull request #7420 from RasmusWL/ssrf-new
Approved by yoff
2 parents de4b655 + 83f87f0 commit 5054d5b

32 files changed

+1871
-0
lines changed

‎docs/codeql/support/reusables/frameworks.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,7 @@ Python built-in support
171171
Twisted, Web framework
172172
Flask-Admin, Web framework
173173
starlette, Asynchronous Server Gateway Interface (ASGI)
174+
requests, HTTP client
174175
dill, Serialization
175176
PyYAML, Serialization
176177
ruamel.yaml, Serialization

‎python/change-notes/2021-12-17-add-SSRF-queries.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
lgtm,codescanning
2+
* Two new queries have been added for detecting Server-side request forgery (SSRF). _Full server-side request forgery_ (`py/full-ssrf`) will only alert when the URL is fully user-controlled, and _Partial server-side request forgery_ (`py/partial-ssrf`) will alert when any part of the URL is user-controlled. Only `py/full-ssrf` will be run by default.
3+
* To support the new SSRF queries, the PyPI package `requests` have been modeled, along with `http.client.HTTP[S]Connection` from the standard library.

‎python/ql/lib/semmle/python/Concepts.qll

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -812,6 +812,72 @@ module HTTP {
812812
}
813813
}
814814
}
815+
816+
/** Provides classes for modeling HTTP clients. */
817+
module Client {
818+
/**
819+
* A data-flow node that makes an outgoing HTTP request.
820+
*
821+
* Extend this class to refine existing API models. If you want to model new APIs,
822+
* extend `HTTP::Client::Request::Range` instead.
823+
*/
824+
class Request extends DataFlow::Node instanceof Request::Range {
825+
/**
826+
* Gets a data-flow node that contributes to the URL of the request.
827+
* Depending on the framework, a request may have multiple nodes which contribute to the URL.
828+
*/
829+
DataFlow::Node getAUrlPart() { result = super.getAUrlPart() }
830+
831+
/** Gets a string that identifies the framework used for this request. */
832+
string getFramework() { result = super.getFramework() }
833+
834+
/**
835+
* Holds if this request is made using a mode that disables SSL/TLS
836+
* certificate validation, where `disablingNode` represents the point at
837+
* which the validation was disabled, and `argumentOrigin` represents the origin
838+
* of the argument that disabled the validation (which could be the same node as
839+
* `disablingNode`).
840+
*/
841+
predicate disablesCertificateValidation(
842+
DataFlow::Node disablingNode, DataFlow::Node argumentOrigin
843+
) {
844+
super.disablesCertificateValidation(disablingNode, argumentOrigin)
845+
}
846+
}
847+
848+
/** Provides a class for modeling new HTTP requests. */
849+
module Request {
850+
/**
851+
* A data-flow node that makes an outgoing HTTP request.
852+
*
853+
* Extend this class to model new APIs. If you want to refine existing API models,
854+
* extend `HTTP::Client::Request` instead.
855+
*/
856+
abstract class Range extends DataFlow::Node {
857+
/**
858+
* Gets a data-flow node that contributes to the URL of the request.
859+
* Depending on the framework, a request may have multiple nodes which contribute to the URL.
860+
*/
861+
abstract DataFlow::Node getAUrlPart();
862+
863+
/** Gets a string that identifies the framework used for this request. */
864+
abstract string getFramework();
865+
866+
/**
867+
* Holds if this request is made using a mode that disables SSL/TLS
868+
* certificate validation, where `disablingNode` represents the point at
869+
* which the validation was disabled, and `argumentOrigin` represents the origin
870+
* of the argument that disabled the validation (which could be the same node as
871+
* `disablingNode`).
872+
*/
873+
abstract predicate disablesCertificateValidation(
874+
DataFlow::Node disablingNode, DataFlow::Node argumentOrigin
875+
);
876+
}
877+
}
878+
// TODO: investigate whether we should treat responses to client requests as
879+
// remote-flow-sources in general.
880+
}
815881
}
816882

817883
/**

‎python/ql/lib/semmle/python/Frameworks.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ private import semmle.python.frameworks.Peewee
3030
private import semmle.python.frameworks.Psycopg2
3131
private import semmle.python.frameworks.Pydantic
3232
private import semmle.python.frameworks.PyMySQL
33+
private import semmle.python.frameworks.Requests
3334
private import semmle.python.frameworks.RestFramework
3435
private import semmle.python.frameworks.Rsa
3536
private import semmle.python.frameworks.RuamelYaml

‎python/ql/lib/semmle/python/frameworks/Requests.qll

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
/**
2+
* Provides classes modeling security-relevant aspects of the `requests` PyPI package.
3+
*
4+
* See
5+
* - https://pypi.org/project/requests/
6+
* - https://docs.python-requests.org/en/latest/
7+
*/
8+
9+
private import python
10+
private import semmle.python.Concepts
11+
private import semmle.python.ApiGraphs
12+
private import semmle.python.dataflow.new.DataFlow
13+
private import semmle.python.dataflow.new.TaintTracking
14+
private import semmle.python.frameworks.internal.InstanceTaintStepsHelper
15+
private import semmle.python.frameworks.Stdlib
16+
17+
/**
18+
* INTERNAL: Do not use.
19+
*
20+
* Provides models for the `requests` PyPI package.
21+
*
22+
* See
23+
* - https://pypi.org/project/requests/
24+
* - https://docs.python-requests.org/en/latest/
25+
*/
26+
private module Requests {
27+
private class OutgoingRequestCall extends HTTP::Client::Request::Range, DataFlow::CallCfgNode {
28+
string methodName;
29+
30+
OutgoingRequestCall() {
31+
methodName in [HTTP::httpVerbLower(), "request"] and
32+
(
33+
this = API::moduleImport("requests").getMember(methodName).getACall()
34+
or
35+
exists(API::Node moduleExporting, API::Node sessionInstance |
36+
moduleExporting in [
37+
API::moduleImport("requests"), //
38+
API::moduleImport("requests").getMember("sessions")
39+
] and
40+
sessionInstance = moduleExporting.getMember(["Session", "session"]).getReturn()
41+
|
42+
this = sessionInstance.getMember(methodName).getACall()
43+
)
44+
)
45+
}
46+
47+
override DataFlow::Node getAUrlPart() {
48+
result = this.getArgByName("url")
49+
or
50+
not methodName = "request" and
51+
result = this.getArg(0)
52+
or
53+
methodName = "request" and
54+
result = this.getArg(1)
55+
}
56+
57+
/** Gets the `verify` argument to this outgoing requests call. */
58+
DataFlow::Node getVerifyArg() { result = this.getArgByName("verify") }
59+
60+
override predicate disablesCertificateValidation(
61+
DataFlow::Node disablingNode, DataFlow::Node argumentOrigin
62+
) {
63+
disablingNode = this.getVerifyArg() and
64+
argumentOrigin = verifyArgBacktracker(disablingNode) and
65+
argumentOrigin.asExpr().(ImmutableLiteral).booleanValue() = false and
66+
not argumentOrigin.asExpr() instanceof None
67+
}
68+
69+
override string getFramework() { result = "requests" }
70+
}
71+
72+
/**
73+
* Extra taint propagation for outgoing requests calls,
74+
* to ensure that responses to user-controlled URL are tainted.
75+
*/
76+
private class OutgoingRequestCallTaintStep extends TaintTracking::AdditionalTaintStep {
77+
override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
78+
nodeFrom = nodeTo.(OutgoingRequestCall).getAUrlPart()
79+
}
80+
}
81+
82+
/** Gets a back-reference to the verify argument `arg`. */
83+
private DataFlow::TypeTrackingNode verifyArgBacktracker(
84+
DataFlow::TypeBackTracker t, DataFlow::Node arg
85+
) {
86+
t.start() and
87+
arg = any(OutgoingRequestCall c).getVerifyArg() and
88+
result = arg.getALocalSource()
89+
or
90+
exists(DataFlow::TypeBackTracker t2 | result = verifyArgBacktracker(t2, arg).backtrack(t2, t))
91+
}
92+
93+
/** Gets a back-reference to the verify argument `arg`. */
94+
private DataFlow::LocalSourceNode verifyArgBacktracker(DataFlow::Node arg) {
95+
result = verifyArgBacktracker(DataFlow::TypeBackTracker::end(), arg)
96+
}
97+
98+
// ---------------------------------------------------------------------------
99+
// Response
100+
// ---------------------------------------------------------------------------
101+
/**
102+
* Provides models for the `requests.models.Response` class
103+
*
104+
* See https://docs.python-requests.org/en/latest/api/#requests.Response.
105+
*/
106+
module Response {
107+
/** Gets a reference to the `requests.models.Response` class. */
108+
private API::Node classRef() {
109+
result = API::moduleImport("requests").getMember("models").getMember("Response")
110+
or
111+
result = API::moduleImport("requests").getMember("Response")
112+
}
113+
114+
/**
115+
* A source of instances of `requests.models.Response`, extend this class to model new instances.
116+
*
117+
* This can include instantiations of the class, return values from function
118+
* calls, or a special parameter that will be set when functions are called by an external
119+
* library.
120+
*
121+
* Use the predicate `Response::instance()` to get references to instances of `requests.models.Response`.
122+
*/
123+
abstract class InstanceSource extends DataFlow::LocalSourceNode { }
124+
125+
/** A direct instantiation of `requests.models.Response`. */
126+
private class ClassInstantiation extends InstanceSource, DataFlow::CallCfgNode {
127+
ClassInstantiation() { this = classRef().getACall() }
128+
}
129+
130+
/** Return value from making a reuqest. */
131+
private class RequestReturnValue extends InstanceSource, DataFlow::Node {
132+
RequestReturnValue() { this = any(OutgoingRequestCall c) }
133+
}
134+
135+
/** Gets a reference to an instance of `requests.models.Response`. */
136+
private DataFlow::TypeTrackingNode instance(DataFlow::TypeTracker t) {
137+
t.start() and
138+
result instanceof InstanceSource
139+
or
140+
exists(DataFlow::TypeTracker t2 | result = instance(t2).track(t2, t))
141+
}
142+
143+
/** Gets a reference to an instance of `requests.models.Response`. */
144+
DataFlow::Node instance() { instance(DataFlow::TypeTracker::end()).flowsTo(result) }
145+
146+
/**
147+
* Taint propagation for `requests.models.Response`.
148+
*/
149+
private class InstanceTaintSteps extends InstanceTaintStepsHelper {
150+
InstanceTaintSteps() { this = "requests.models.Response" }
151+
152+
override DataFlow::Node getInstance() { result = instance() }
153+
154+
override string getAttributeName() {
155+
result in ["text", "content", "raw", "links", "cookies", "headers"]
156+
}
157+
158+
override string getMethodName() { result in ["json", "iter_content", "iter_lines"] }
159+
160+
override string getAsyncMethodName() { none() }
161+
}
162+
163+
/** An attribute read that is a file-like instance. */
164+
private class FileLikeInstances extends Stdlib::FileLikeObject::InstanceSource {
165+
FileLikeInstances() {
166+
this.(DataFlow::AttrRead).getObject() = instance() and
167+
this.(DataFlow::AttrRead).getAttributeName() = "raw"
168+
}
169+
}
170+
}
171+
}

0 commit comments

Comments
 (0)