Skip to content

Commit ff07d22

Browse files
paldepindpaldepind
committed
WIP
1 parent c5cb86a commit ff07d22

File tree

13 files changed

+2444
-17
lines changed

13 files changed

+2444
-17
lines changed

‎rust/ql/lib/codeql/rust/frameworks/reqwest.model.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ extensions:
99
pack: codeql/rust-all
1010
extensible: sinkModel
1111
data:
12-
- ["<reqwest::async_impl::client::Client>::request", "Argument[1]", "transmission", "manual"]
13-
- ["<reqwest::blocking::client::Client>::request", "Argument[1]", "transmission", "manual"]
12+
- ["<reqwest::async_impl::client::Client>::request", "Argument[1]", "request-url", "manual"]
13+
- ["<reqwest::blocking::client::Client>::request", "Argument[1]", "request-url", "manual"]
1414
- addsTo:
1515
pack: codeql/rust-all
1616
extensible: summaryModel

‎rust/ql/lib/codeql/rust/security/CleartextTransmissionExtensions.qll‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,6 @@ module CleartextTransmission {
5353
* A sink defined through MaD.
5454
*/
5555
private class ModelsAsDataSink extends Sink {
56-
ModelsAsDataSink() { sinkNode(this, "transmission") }
56+
ModelsAsDataSink() { sinkNode(this, ["transmission", "request-url"]) }
5757
}
5858
}

‎rust/ql/lib/codeql/rust/security/RequestForgeryExtensions.qll‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
/**
2+
* Provides classes and predicates for reasoning about request forgery
3+
* vulnerabilities.
4+
*/
5+
6+
import rust
7+
private import codeql.rust.dataflow.DataFlow
8+
private import codeql.rust.dataflow.FlowSink
9+
private import codeql.rust.dataflow.FlowSource
10+
private import codeql.rust.Concepts
11+
private import codeql.rust.security.CleartextTransmissionExtensions
12+
13+
/**
14+
* Provides default sources, sinks and barriers for detecting request forgery
15+
* vulnerabilities, as well as extension points for adding your own.
16+
*/
17+
module RequestForgery {
18+
/**
19+
* A data flow source for request forgery vulnerabilities.
20+
*/
21+
abstract class Source extends DataFlow::Node { }
22+
23+
/**
24+
* A data flow sink for request forgery vulnerabilities.
25+
*/
26+
abstract class Sink extends QuerySink::Range {
27+
/**
28+
* Gets the name of a part of the request that may be tainted by this sink,
29+
* such as the URL or the host.
30+
*/
31+
override string getSinkType() { result = "RequestForgery" }
32+
}
33+
34+
/**
35+
* A barrier for request forgery vulnerabilities.
36+
*/
37+
abstract class Barrier extends DataFlow::Node { }
38+
39+
/**
40+
* An active threat-model source, considered as a flow source.
41+
*/
42+
private class ActiveThreatModelSourceAsSource extends Source, ActiveThreatModelSource { }
43+
44+
// TODO: Do this in a cleaner way
45+
// private class ClearTextTransmissionSink extends Sink instanceof CleartextTransmission::Sink { }
46+
/**
47+
* A sink for request forgery from model data.
48+
*/
49+
private class ModelsAsDataSink extends Sink {
50+
ModelsAsDataSink() { sinkNode(this, "request-url") }
51+
}
52+
}

‎rust/ql/lib/ext/generated/reqwest.model.yml‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -424,21 +424,21 @@ extensions:
424424
pack: codeql/rust-all
425425
extensible: sinkModel
426426
data:
427-
- ["<reqwest::async_impl::client::Client>::delete", "Argument[0]", "transmission", "df-generated"]
428-
- ["<reqwest::async_impl::client::Client>::get", "Argument[0]", "transmission", "df-generated"]
429-
- ["<reqwest::async_impl::client::Client>::head", "Argument[0]", "transmission", "df-generated"]
430-
- ["<reqwest::async_impl::client::Client>::patch", "Argument[0]", "transmission", "df-generated"]
431-
- ["<reqwest::async_impl::client::Client>::post", "Argument[0]", "transmission", "df-generated"]
432-
- ["<reqwest::async_impl::client::Client>::put", "Argument[0]", "transmission", "df-generated"]
427+
- ["<reqwest::async_impl::client::Client>::delete", "Argument[0]", "request-url", "df-generated"]
428+
- ["<reqwest::async_impl::client::Client>::get", "Argument[0]", "request-url", "df-generated"]
429+
- ["<reqwest::async_impl::client::Client>::head", "Argument[0]", "request-url", "df-generated"]
430+
- ["<reqwest::async_impl::client::Client>::patch", "Argument[0]", "request-url", "df-generated"]
431+
- ["<reqwest::async_impl::client::Client>::post", "Argument[0]", "request-url", "df-generated"]
432+
- ["<reqwest::async_impl::client::Client>::put", "Argument[0]", "request-url", "df-generated"]
433433
- ["<reqwest::async_impl::multipart::Form>::into_stream", "Argument[self]", "log-injection", "df-generated"]
434434
- ["<reqwest::async_impl::multipart::Form>::stream", "Argument[self]", "log-injection", "df-generated"]
435435
- ["<reqwest::async_impl::request::RequestBuilder>::multipart", "Argument[0]", "log-injection", "df-generated"]
436-
- ["<reqwest::blocking::client::Client>::delete", "Argument[0]", "transmission", "df-generated"]
437-
- ["<reqwest::blocking::client::Client>::get", "Argument[0]", "transmission", "df-generated"]
438-
- ["<reqwest::blocking::client::Client>::head", "Argument[0]", "transmission", "df-generated"]
439-
- ["<reqwest::blocking::client::Client>::patch", "Argument[0]", "transmission", "df-generated"]
440-
- ["<reqwest::blocking::client::Client>::post", "Argument[0]", "transmission", "df-generated"]
441-
- ["<reqwest::blocking::client::Client>::put", "Argument[0]", "transmission", "df-generated"]
436+
- ["<reqwest::blocking::client::Client>::delete", "Argument[0]", "request-url", "df-generated"]
437+
- ["<reqwest::blocking::client::Client>::get", "Argument[0]", "request-url", "df-generated"]
438+
- ["<reqwest::blocking::client::Client>::head", "Argument[0]", "request-url", "df-generated"]
439+
- ["<reqwest::blocking::client::Client>::patch", "Argument[0]", "request-url", "df-generated"]
440+
- ["<reqwest::blocking::client::Client>::post", "Argument[0]", "request-url", "df-generated"]
441+
- ["<reqwest::blocking::client::Client>::put", "Argument[0]", "request-url", "df-generated"]
442442
- ["<reqwest::blocking::multipart::Form>::into_reader", "Argument[self]", "log-injection", "df-generated"]
443443
- ["<reqwest::blocking::multipart::Form>::reader", "Argument[self]", "log-injection", "df-generated"]
444444
- ["<reqwest::blocking::multipart::Reader as std::io::Read>::read", "Argument[self]", "log-injection", "df-generated"]
@@ -450,9 +450,9 @@ extensions:
450450
- ["<reqwest::blocking::response::Response>::text_with_charset", "Argument[self]", "pointer-access", "df-generated"]
451451
- ["<reqwest::connect::ConnectorService as tower_service::Service>::call", "Argument[0]", "log-injection", "df-generated"]
452452
- ["<reqwest::error::Error>::new", "Argument[1]", "pointer-access", "df-generated"]
453-
- ["reqwest::blocking::get", "Argument[0]", "transmission", "df-generated"]
453+
- ["reqwest::blocking::get", "Argument[0]", "request-url", "df-generated"]
454454
- ["reqwest::blocking::wait::timeout", "Argument[1]", "pointer-access", "df-generated"]
455-
- ["reqwest::get", "Argument[0]", "transmission", "df-generated"]
455+
- ["reqwest::get", "Argument[0]", "request-url", "df-generated"]
456456
- addsTo:
457457
pack: codeql/rust-all
458458
extensible: sourceModel

‎rust/ql/src/queries/security/CWE-918/RequestForgery.qhelp‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>
8+
Directly incorporating user input into an HTTP request without validating the
9+
input can facilitate server-side request forgery (SSRF) attacks. In these
10+
attacks, the server may be tricked into making a request to an unintended API
11+
endpoint or resource.
12+
13+
If the server performing the request is connected to an internal network, this
14+
can give an attacker the means to bypass the network boundary and make requests
15+
against internal services.
16+
17+
A forged request may perform an unintended action on behalf of the attacker, or
18+
cause information leak if redirected to an external server or if the request
19+
response is fed back to the user. It may also compromise the server making the
20+
request, if the request response is handled in an unsafe way.
21+
</p>
22+
</overview>
23+
24+
<recommendation>
25+
<p>
26+
To guard against SSRF attacks you should avoid putting user-provided input
27+
directly into a request URL. Instead, maintain a list of authorized URLs on the
28+
server; then choose from that list based on the input provided. Alternatively,
29+
ensure requests constructed from user input are limited to a particular host or
30+
more restrictive URL prefix.
31+
</p>
32+
</recommendation>
33+
34+
<example>
35+
<p>
36+
The following example shows an HTTP request parameter being used directly to
37+
form a new request without validating the input, which facilitates SSRF attacks.
38+
It also shows how to remedy the problem by validating the user input against a
39+
known fixed string.
40+
</p>
41+
42+
<sample src="RequestForgery.rs" />
43+
</example>
44+
45+
<references>
46+
<li>
47+
<a href="https://owasp.org/www-community/attacks/Server_Side_Request_Forgery">OWASP SSRF</a>
48+
</li>
49+
<li>
50+
<a href="https://cwe.mitre.org/data/definitions/918.html">CWE-918: Server-Side Request Forgery (SSRF)</a>
51+
</li>
52+
</references>
53+
54+
</qhelp>

‎rust/ql/src/queries/security/CWE-918/RequestForgery.ql‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/**
2+
* @name Server-side request forgery
3+
* @description Making a network request with user-controlled data in the URL allows for request forgery attacks.
4+
* @kind path-problem
5+
* @problem.severity error
6+
* @security-severity 9.1
7+
* @precision high
8+
* @id rust/request-forgery
9+
* @tags security
10+
* external/cwe/cwe-918
11+
*/
12+
13+
import rust
14+
import codeql.rust.dataflow.DataFlow
15+
import codeql.rust.dataflow.TaintTracking
16+
private import codeql.rust.dataflow.DataFlow
17+
private import codeql.rust.dataflow.FlowSink
18+
private import codeql.rust.Concepts
19+
private import codeql.rust.security.CleartextTransmissionExtensions
20+
private import codeql.rust.security.RequestForgeryExtensions
21+
22+
/**
23+
* A taint-tracking configuration for detecting request forgery vulnerabilities.
24+
*/
25+
module RequestForgeryConfig implements DataFlow::ConfigSig {
26+
predicate isSource(DataFlow::Node source) { source instanceof RequestForgery::Source }
27+
28+
predicate isSink(DataFlow::Node sink) { sink instanceof RequestForgery::Sink }
29+
30+
predicate isBarrier(DataFlow::Node barrier) { barrier instanceof RequestForgery::Barrier }
31+
32+
predicate observeDiffInformedIncrementalMode() { any() }
33+
}
34+
35+
module RequestForgeryFlow = TaintTracking::Global<RequestForgeryConfig>;
36+
37+
import RequestForgeryFlow::PathGraph
38+
39+
from RequestForgeryFlow::PathNode source, RequestForgeryFlow::PathNode sink
40+
where RequestForgeryFlow::flowPath(source, sink)
41+
select sink.getNode(), source, sink, "The $@ of this request depends on a $@.", sink, "URL",
42+
source.getNode(), "user-provided value"

‎rust/ql/src/queries/security/CWE-918/RequestForgery.rs‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// BAD: Endpoint handler that makes requests based on user input
2+
async fn vulnerable_endpoint_handler(req: Request) -> Result<Response> {
3+
// This request is vulnerable to SSRF attacks as the user controls the
4+
// entire URL
5+
let response = reqwest::get(&req.user_url).await;
6+
7+
match response {
8+
Ok(resp) => {
9+
let body = resp.text().await.unwrap_or_default();
10+
Ok(Response {
11+
message: "Success".to_string(),
12+
data: body,
13+
})
14+
}
15+
Err(_) => Err("Request failed")
16+
}
17+
}
18+
19+
// GOOD: Validate user input against an allowlist
20+
async fn secure_endpoint_handler(req: Request) -> Result<Response> {
21+
// Allow list of specific, known-safe URLs
22+
let allowed_hosts = ["api.example.com", "trusted-service.com"];
23+
24+
if !allowed_hosts.contains(&req.user_url) {
25+
return Err("Untrusted domain");
26+
}
27+
// This request is safe as the user input has been validated
28+
let response = reqwest::get(&req.user_url).await;
29+
match response {
30+
Ok(resp) => {
31+
let body = resp.text().await.unwrap_or_default();
32+
Ok(Response {
33+
message: "Success".to_string(),
34+
data: body,
35+
})
36+
}
37+
Err(_) => Err("Request failed")
38+
}
39+
}

0 commit comments

Comments
 (0)