Skip to content

Commit b9c0816

Browse files
author
Porcuiney Hairs
committed
C# : Add query to detect SSRF
1 parent ee46717 commit b9c0816

File tree

7 files changed

+386
-0
lines changed

7 files changed

+386
-0
lines changed

‎csharp/ql/src/experimental/CWE-918/RequestForgery.cs

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
namespace RequestForgery.Controllers
2+
{
3+
public class SSRFController : Controller
4+
{
5+
[HttpPost]
6+
[ValidateAntiForgeryToken]
7+
public async Task<ActionResult> Bad(string url)
8+
{
9+
var request = new HttpRequestMessage(HttpMethod.Get, url);
10+
11+
var client = new HttpClient();
12+
await client.SendAsync(request);
13+
14+
return View();
15+
}
16+
17+
[HttpPost]
18+
[ValidateAntiForgeryToken]
19+
public async Task<ActionResult> Good(string url)
20+
{
21+
string baseUrl = "www.mysecuresite.com/";
22+
if (url.StartsWith(baseUrl))
23+
{
24+
var request = new HttpRequestMessage(HttpMethod.Get, url);
25+
var client = new HttpClient();
26+
await client.SendAsync(request);
27+
28+
}
29+
30+
return View();
31+
}
32+
}
33+
}

‎csharp/ql/src/experimental/CWE-918/RequestForgery.qhelp

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
4+
5+
<overview>
6+
<p>Directly incorporating user input into a HTTP request without validating the input
7+
can facilitate Server Side Request Forgery (SSRF) attacks. In these attacks, the server
8+
may be tricked into making a request and interacting with an attacker-controlled server.
9+
</p>
10+
11+
</overview>
12+
<recommendation>
13+
14+
<p>To guard against SSRF attacks, it is advisable to avoid putting user input
15+
directly into the request URL. Instead, maintain a list of authorized
16+
URLs on the server; then choose from that list based on the user input provided.</p>
17+
18+
</recommendation>
19+
<example>
20+
21+
<p>The following example shows an HTTP request parameter being used directly in a forming a
22+
new request without validating the input, which facilitates SSRF attacks.
23+
It also shows how to remedy the problem by validating the user input against a known fixed string.
24+
</p>
25+
26+
<sample src="RequestForgery.cs" />
27+
28+
</example>
29+
<references>
30+
<li>
31+
<a href="https://owasp.org/www-community/attacks/Server_Side_Request_Forgery">OWASP SSRF</a>
32+
</li>
33+
34+
</references>
35+
</qhelp>

‎csharp/ql/src/experimental/CWE-918/RequestForgery.ql

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/**
2+
* @name Uncontrolled data used in a WebClient
3+
* @description The WebClient class allows developers to request resources,
4+
* accessing resources influenced by users can allow an attacker to access local files.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @precision high
8+
* @id cs/request-forgery
9+
* @tags security
10+
* external/cwe/cwe-918
11+
*/
12+
13+
import csharp
14+
import RequestForgery::RequestForgery
15+
import semmle.code.csharp.dataflow.DataFlow::DataFlow::PathGraph
16+
17+
from RequestForgeryConfiguration c, DataFlow::PathNode source, DataFlow::PathNode sink
18+
where c.hasFlowPath(source, sink)
19+
select sink.getNode(), source, sink, "$@ flows to here and is used in a method of WebClient.",
20+
source.getNode(), "User-provided value"

‎csharp/ql/src/experimental/CWE-918/RequestForgery.qll

Lines changed: 232 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,232 @@
1+
import csharp
2+
3+
module RequestForgery {
4+
import semmle.code.csharp.controlflow.Guards
5+
import semmle.code.csharp.frameworks.System
6+
import semmle.code.csharp.frameworks.system.Web
7+
import semmle.code.csharp.frameworks.Format
8+
import semmle.code.csharp.security.dataflow.flowsources.Remote
9+
10+
/**
11+
* A data flow source for server side request forgery vulnerabilities.
12+
*/
13+
abstract private class Source extends DataFlow::Node { }
14+
15+
/**
16+
* A data flow sink for server side request forgery vulnerabilities.
17+
*/
18+
abstract private class Sink extends DataFlow::ExprNode { }
19+
20+
/**
21+
* A data flow BarrierGuard which blocks the flow of taint for
22+
* server side request forgery vulnerabilities.
23+
*/
24+
abstract private class BarrierGuard extends DataFlow::BarrierGuard { }
25+
26+
/**
27+
* A taint-tracking configuration for detecting server side request forgery vulnerabilities.
28+
*/
29+
class RequestForgeryConfiguration extends DataFlow::Configuration {
30+
RequestForgeryConfiguration() { this = "Server Side Request forgery" }
31+
32+
override predicate isSource(DataFlow::Node source) { source instanceof Source }
33+
34+
override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
35+
36+
override predicate isAdditionalFlowStep(DataFlow::Node prev, DataFlow::Node succ) {
37+
interpolatedStringFlowStep(prev, succ)
38+
or
39+
stringReplaceStep(prev, succ)
40+
or
41+
uriCreationStep(prev, succ)
42+
or
43+
formatConvertStep(prev, succ)
44+
or
45+
toStringStep(prev, succ)
46+
or
47+
stringConcatStep(prev, succ)
48+
or
49+
stringFormatStep(prev, succ)
50+
or
51+
pathCombineStep(prev, succ)
52+
}
53+
54+
override predicate isBarrierGuard(DataFlow::BarrierGuard guard) {
55+
guard instanceof BarrierGuard
56+
}
57+
}
58+
59+
/**
60+
* A remote data flow source taken as a source
61+
* for Server Side Request Forgery(SSRF) Vulnerabilities.
62+
*/
63+
private class RemoteFlowSourceAsSource extends Source {
64+
RemoteFlowSourceAsSource() { this instanceof RemoteFlowSource }
65+
}
66+
67+
/**
68+
* An url argument to a `HttpRequestMessage` constructor call
69+
* taken as a sink for Server Side Request Forgery(SSRF) Vulnerabilities.
70+
*/
71+
private class SystemWebHttpRequestMessageSink extends Sink {
72+
SystemWebHttpRequestMessageSink() {
73+
exists(Class c | c.hasQualifiedName("System.Net.Http.HttpRequestMessage") |
74+
c.getAConstructor().getACall().getArgument(1) = this.asExpr()
75+
)
76+
}
77+
}
78+
79+
/**
80+
* An argument to a `WebRequest.Create` call taken as a
81+
* sink for Server Side Request Forgery(SSRF) Vulnerabilities. *
82+
*/
83+
private class SystemNetWebRequestCreateSink extends Sink {
84+
SystemNetWebRequestCreateSink() {
85+
exists(Method m |
86+
m.getDeclaringType().hasQualifiedName("System.Net.WebRequest") and m.hasName("Create")
87+
|
88+
m.getACall().getArgument(0) = this.asExpr()
89+
)
90+
}
91+
}
92+
93+
/**
94+
* An argument to a new HTTP Request call of a `System.Net.Http.HttpClient` object
95+
* taken as a sink for Server Side Request Forgery(SSRF) Vulnerabilities.
96+
*/
97+
private class SystemNetHttpClientSink extends Sink {
98+
SystemNetHttpClientSink() {
99+
exists(Method m |
100+
m.getDeclaringType().hasQualifiedName("System.Net.Http.HttpClient") and
101+
m.hasName([
102+
"DeleteAsync", "GetAsync", "GetByteArrayAsync", "GetStreamAsync", "GetStringAsync",
103+
"PatchAsync", "PostAsync", "PutAsync"
104+
])
105+
|
106+
m.getACall().getArgument(0) = this.asExpr()
107+
)
108+
}
109+
}
110+
111+
/**
112+
* An url argument to a method call of a `System.Net.WebClient` object
113+
* taken as a sink for Server Side Request Forgery(SSRF) Vulnerabilities.
114+
*/
115+
private class SystemNetClientBaseAddressSink extends Sink {
116+
SystemNetClientBaseAddressSink() {
117+
exists(Property p |
118+
p.hasName("BaseAddress") and
119+
p.getDeclaringType()
120+
.hasQualifiedName(["System.Net.WebClient", "System.Net.Http.HttpClient"])
121+
|
122+
p.getAnAssignedValue() = this.asExpr()
123+
)
124+
}
125+
}
126+
127+
/**
128+
* A method call which checks the base of the tainted uri is assumed
129+
* to be a guard for Server Side Request Forgery(SSRF) Vulnerabilities.
130+
* This guard considers all checks as valid.
131+
*/
132+
private class BaseUriGuard extends BarrierGuard, MethodCall {
133+
BaseUriGuard() { this.getTarget().hasQualifiedName("System.Uri", "IsBaseOf") }
134+
135+
override predicate checks(Expr e, AbstractValue v) {
136+
// we consider any checks against the tainted value to sainitize the taint.
137+
// This implies any check such as shown below block the taint flow.
138+
// Uri url = new Uri("whitelist.com")
139+
// if (url.isBaseOf(`taint1))
140+
(e = this.getArgument(0) or e = this.getQualifier()) and
141+
v.(AbstractValues::BooleanValue).getValue() = true
142+
}
143+
}
144+
145+
/**
146+
* A method call which checks if the Uri starts with a white-listed string is assumed
147+
* to be a guard for Server Side Request Forgery(SSRF) Vulnerabilities.
148+
* This guard considers all checks as valid.
149+
*/
150+
private class StringStartsWithBarrierGuard extends BarrierGuard, MethodCall {
151+
StringStartsWithBarrierGuard() { this.getTarget().hasQualifiedName("System.String.StartsWith") }
152+
153+
override predicate checks(Expr e, AbstractValue v) {
154+
// Any check such as the ones shown below
155+
// "https://myurl.com/".startsWith(`taint`)
156+
// `taint`.startsWith("https://myurl.com/")
157+
// are assumed to sainitize the taint
158+
(e = this.getQualifier() or this.getArgument(0) = e) and
159+
v.(AbstractValues::BooleanValue).getValue() = true
160+
}
161+
}
162+
163+
private predicate stringFormatStep(DataFlow::Node prev, DataFlow::Node succ) {
164+
exists(FormatCall c | c.getArgument(0) = prev.asExpr() and c = succ.asExpr())
165+
}
166+
167+
private predicate pathCombineStep(DataFlow::Node prev, DataFlow::Node succ) {
168+
exists(MethodCall combineCall |
169+
combineCall.getTarget().hasQualifiedName("System.IO.Path", "Combine") and
170+
combineCall.getArgument(0) = prev.asExpr() and
171+
combineCall = succ.asExpr()
172+
)
173+
}
174+
175+
private predicate uriCreationStep(DataFlow::Node prev, DataFlow::Node succ) {
176+
exists(ObjectCreation oc |
177+
oc.getTarget().getDeclaringType().hasQualifiedName("System.Uri") and
178+
oc.getArgument(0) = prev.asExpr() and
179+
oc = succ.asExpr()
180+
)
181+
}
182+
183+
private predicate interpolatedStringFlowStep(DataFlow::Node prev, DataFlow::Node succ) {
184+
exists(InterpolatedStringExpr i |
185+
// allow `$"http://{`taint`}/blabla/");"` or
186+
// allow `$"https://{`taint`}/blabla/");"`
187+
i.getText(0).getValue().matches(["http://", "http://"]) and
188+
i.getInsert(1) = prev.asExpr() and
189+
succ.asExpr() = i
190+
or
191+
// allow `$"{`taint`}/blabla/");"`
192+
i.getInsert(0) = prev.asExpr() and
193+
succ.asExpr() = i
194+
)
195+
}
196+
197+
private predicate stringReplaceStep(DataFlow::Node prev, DataFlow::Node succ) {
198+
exists(MethodCall mc, SystemStringClass s |
199+
mc = s.getReplaceMethod().getACall() and
200+
mc.getQualifier() = prev.asExpr() and
201+
succ.asExpr() = mc
202+
)
203+
}
204+
205+
private predicate stringConcatStep(DataFlow::Node prev, DataFlow::Node succ) {
206+
exists(AddExpr a |
207+
a.getLeftOperand() = prev.asExpr()
208+
or
209+
a.getRightOperand() = prev.asExpr() and
210+
a.getLeftOperand().(StringLiteral).getValue() = ["http://", "https://"]
211+
|
212+
a = succ.asExpr()
213+
)
214+
}
215+
216+
private predicate formatConvertStep(DataFlow::Node prev, DataFlow::Node succ) {
217+
exists(Method m |
218+
m.hasQualifiedName("System.Convert",
219+
["FromBase64String", "FromHexString", "FromBase64CharArray"]) and
220+
m.getParameter(0) = prev.asParameter() and
221+
succ.asExpr() = m.getACall()
222+
)
223+
}
224+
225+
private predicate toStringStep(DataFlow::Node prev, DataFlow::Node succ) {
226+
exists(MethodCall ma |
227+
ma.getTarget().hasName("ToString") and
228+
ma.getQualifier() = prev.asExpr() and
229+
succ.asExpr() = ma
230+
)
231+
}
232+
}

‎csharp/ql/test/experimental/CWE-918/RequestForgery.cs

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
// semmle-extractor-options: ${testdir}/../../resources/stubs/System.Web.cs /r:System.Threading.Tasks.dll /r:System.Collections.Specialized.dll /r:System.Runtime.dll /r:System.Private.Uri.dll
2+
3+
using System;
4+
using System.Threading.Tasks;
5+
using System.Web.Mvc;
6+
using System.Net.Http;
7+
8+
namespace RequestForgery.Controllers
9+
{
10+
public class SSRFController : Controller
11+
{
12+
[HttpPost]
13+
[ValidateAntiForgeryToken]
14+
public async Task<ActionResult> Bad(string url)
15+
{
16+
var request = new HttpRequestMessage(HttpMethod.Get, url);
17+
18+
var client = new HttpClient();
19+
await client.SendAsync(request);
20+
21+
return View();
22+
}
23+
24+
[HttpPost]
25+
[ValidateAntiForgeryToken]
26+
public async Task<ActionResult> Good(string url)
27+
{
28+
string baseUrl = "www.mysecuresite.com/";
29+
if (url.StartsWith(baseUrl))
30+
{
31+
var request = new HttpRequestMessage(HttpMethod.Get, url);
32+
var client = new HttpClient();
33+
await client.SendAsync(request);
34+
35+
}
36+
37+
return View();
38+
}
39+
}
40+
}
41+
// Missing stubs:
42+
namespace System.Net.Http
43+
{
44+
public class HttpClient
45+
{
46+
public async Task SendAsync(HttpRequestMessage request) => throw null;
47+
}
48+
49+
public class HttpRequestMessage
50+
{
51+
public HttpRequestMessage(HttpMethod method, string requestUri) => throw null;
52+
}
53+
54+
public class HttpMethod
55+
{
56+
public static readonly HttpMethod Get;
57+
}
58+
}

‎csharp/ql/test/experimental/CWE-918/RequestForgery.expected

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
edges
2+
| RequestForgery.cs:14:52:14:54 | url : String | RequestForgery.cs:16:66:16:68 | access to parameter url |
3+
nodes
4+
| RequestForgery.cs:14:52:14:54 | url : String | semmle.label | url : String |
5+
| RequestForgery.cs:16:66:16:68 | access to parameter url | semmle.label | access to parameter url |
6+
#select
7+
| RequestForgery.cs:16:66:16:68 | access to parameter url | RequestForgery.cs:14:52:14:54 | url : String | RequestForgery.cs:16:66:16:68 | access to parameter url | $@ flows to here and is used in a method of WebClient. | RequestForgery.cs:14:52:14:54 | url | User-provided value |

‎csharp/ql/test/experimental/CWE-918/RequestForgery.qlref

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

0 commit comments

Comments
 (0)