Skip to content

Commit af49301

Browse files
authored
Merge pull request #20381 from paldepind/rust/request-forgery-query
Rust: Add basic request forgery query
2 parents 00f6448 + 50bdc65 commit af49301

20 files changed

+2383
-21
lines changed

rust/ql/integration-tests/query-suite/rust-code-scanning.qls.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ ql/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.ql
1919
ql/rust/ql/src/queries/security/CWE-770/UncontrolledAllocationSize.ql
2020
ql/rust/ql/src/queries/security/CWE-798/HardcodedCryptographicValue.ql
2121
ql/rust/ql/src/queries/security/CWE-825/AccessInvalidPointer.ql
22+
ql/rust/ql/src/queries/security/CWE-918/RequestForgery.ql
2223
ql/rust/ql/src/queries/summary/LinesOfCode.ql
2324
ql/rust/ql/src/queries/summary/LinesOfUserCode.ql
2425
ql/rust/ql/src/queries/summary/NodesWithTypeAtLengthLimit.ql

rust/ql/integration-tests/query-suite/rust-security-and-quality.qls.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ ql/rust/ql/src/queries/security/CWE-770/UncontrolledAllocationSize.ql
2222
ql/rust/ql/src/queries/security/CWE-798/HardcodedCryptographicValue.ql
2323
ql/rust/ql/src/queries/security/CWE-825/AccessAfterLifetime.ql
2424
ql/rust/ql/src/queries/security/CWE-825/AccessInvalidPointer.ql
25+
ql/rust/ql/src/queries/security/CWE-918/RequestForgery.ql
2526
ql/rust/ql/src/queries/summary/LinesOfCode.ql
2627
ql/rust/ql/src/queries/summary/LinesOfUserCode.ql
2728
ql/rust/ql/src/queries/summary/NodesWithTypeAtLengthLimit.ql

rust/ql/integration-tests/query-suite/rust-security-extended.qls.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ ql/rust/ql/src/queries/security/CWE-770/UncontrolledAllocationSize.ql
2121
ql/rust/ql/src/queries/security/CWE-798/HardcodedCryptographicValue.ql
2222
ql/rust/ql/src/queries/security/CWE-825/AccessAfterLifetime.ql
2323
ql/rust/ql/src/queries/security/CWE-825/AccessInvalidPointer.ql
24+
ql/rust/ql/src/queries/security/CWE-918/RequestForgery.ql
2425
ql/rust/ql/src/queries/summary/LinesOfCode.ql
2526
ql/rust/ql/src/queries/summary/LinesOfUserCode.ql
2627
ql/rust/ql/src/queries/summary/NodesWithTypeAtLengthLimit.ql

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
}
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
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+
12+
/**
13+
* Provides default sources, sinks and barriers for detecting request forgery
14+
* vulnerabilities, as well as extension points for adding your own.
15+
*/
16+
module RequestForgery {
17+
/**
18+
* A data flow source for request forgery vulnerabilities.
19+
*/
20+
abstract class Source extends DataFlow::Node { }
21+
22+
/**
23+
* A data flow sink for request forgery vulnerabilities.
24+
*/
25+
abstract class Sink extends QuerySink::Range {
26+
/**
27+
* Gets the name of a part of the request that may be tainted by this sink,
28+
* such as the URL or the host.
29+
*/
30+
override string getSinkType() { result = "RequestForgery" }
31+
}
32+
33+
/**
34+
* A barrier for request forgery vulnerabilities.
35+
*/
36+
abstract class Barrier extends DataFlow::Node { }
37+
38+
/**
39+
* An active threat-model source, considered as a flow source.
40+
*/
41+
private class ActiveThreatModelSourceAsSource extends Source, ActiveThreatModelSource { }
42+
43+
/**
44+
* A sink for request forgery from model data.
45+
*/
46+
private class ModelsAsDataSink extends Sink {
47+
ModelsAsDataSink() { sinkNode(this, "request-url") }
48+
}
49+
}

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
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: newQuery
3+
---
4+
* Added a new query, `rust/request-forgery`, for detecting server-side request forgery vulnerabilities.
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
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 is connected to an internal network, attackers can bypass security
14+
boundaries to target internal services.
15+
16+
Forged requests can execute unintended actions, leak data if redirected to an
17+
external server, or compromise the server if responses are handled insecurely.
18+
</p>
19+
</overview>
20+
21+
<recommendation>
22+
<p>
23+
To guard against SSRF attacks, you should avoid putting user-provided input
24+
directly into a request URL. Instead, maintain a list of authorized URLs on the
25+
server; then choose from that list based on the input provided. Alternatively,
26+
ensure requests constructed from user input are limited to a particular host or
27+
a more restrictive URL prefix.
28+
</p>
29+
</recommendation>
30+
31+
<example>
32+
<p>
33+
The following example shows an HTTP request parameter being used directly to
34+
form a new request without validating the input, which facilitates SSRF attacks.
35+
It also shows how to remedy the problem by validating the user input against a
36+
known fixed string.
37+
</p>
38+
39+
<sample src="RequestForgery.rs" />
40+
</example>
41+
42+
<references>
43+
<li>
44+
<a href="https://owasp.org/www-community/attacks/Server_Side_Request_Forgery">OWASP Server Side Request Forgery</a>.
45+
</li>
46+
</references>
47+
48+
</qhelp>
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
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+
private import rust
14+
private import codeql.rust.dataflow.TaintTracking
15+
private import codeql.rust.dataflow.DataFlow
16+
private import codeql.rust.security.RequestForgeryExtensions
17+
18+
/**
19+
* A taint-tracking configuration for detecting request forgery vulnerabilities.
20+
*/
21+
module RequestForgeryConfig implements DataFlow::ConfigSig {
22+
predicate isSource(DataFlow::Node source) { source instanceof RequestForgery::Source }
23+
24+
predicate isSink(DataFlow::Node sink) { sink instanceof RequestForgery::Sink }
25+
26+
predicate isBarrier(DataFlow::Node barrier) { barrier instanceof RequestForgery::Barrier }
27+
28+
predicate observeDiffInformedIncrementalMode() { any() }
29+
}
30+
31+
module RequestForgeryFlow = TaintTracking::Global<RequestForgeryConfig>;
32+
33+
import RequestForgeryFlow::PathGraph
34+
35+
from RequestForgeryFlow::PathNode source, RequestForgeryFlow::PathNode sink
36+
where RequestForgeryFlow::flowPath(source, sink)
37+
select sink.getNode(), source, sink, "The URL of this request depends on a $@.", source.getNode(),
38+
"user-provided value"

0 commit comments

Comments
 (0)