Skip to content

Commit 2ad59a5

Browse files
am0o0am0o0
committed
fix SSRF sinks
1 parent c361caf commit 2ad59a5

3 files changed

Lines changed: 28 additions & 140 deletions

File tree

‎go/ql/lib/ext/github.com.valyala.fasthttp.model.yml‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,4 @@ extensions:
88
- ["github.com/valyala/fasthttp", "URI", False, "Update", "", "", "Argument[0]", "Argument[-1]", "taint", "manual"]
99
- ["github.com/valyala/fasthttp", "URI", False, "UpdateBytes", "", "", "Argument[0]", "Argument[-1]", "taint", "manual"]
1010
- ["github.com/valyala/fasthttp", "URI", False, "Parse", "", "", "Argument[0]", "Argument[-1]", "taint", "manual"]
11-
- ["github.com/valyala/fasthttp", "URI", False, "Parse", "", "", "Argument[1]", "Argument[-1]", "taint", "manual"]
12-
- ["github.com/valyala/fasthttp", "Request", False, "SetRequestURI", "", "", "Argument[0]", "Argument[-1]", "taint", "manual"]
13-
- ["github.com/valyala/fasthttp", "Request", False, "SetRequestURIBytes", "", "", "Argument[0]", "Argument[-1]", "taint", "manual"]
14-
- ["github.com/valyala/fasthttp", "Request", False, "SetURI", "", "", "Argument[0]", "Argument[-1]", "taint", "manual"]
15-
- ["github.com/valyala/fasthttp", "Request", False, "String", "", "", "Argument[0]", "Argument[-1]", "taint", "manual"]
16-
- ["github.com/valyala/fasthttp", "Request", False, "SetHost", "", "", "Argument[0]", "Argument[-1]", "taint", "manual"]
17-
- ["github.com/valyala/fasthttp", "Request", False, "SetHost", "", "", "Argument[0]", "Argument[-1]", "taint", "manual"]
18-
- ["github.com/valyala/fasthttp", "Request", False, "SetHostBytes", "", "", "Argument[0]", "Argument[-1]", "taint", "manual"]
11+
- ["github.com/valyala/fasthttp", "URI", False, "Parse", "", "", "Argument[1]", "Argument[-1]", "taint", "manual"]

‎go/ql/lib/semmle/go/frameworks/Fasthttp.qll‎

Lines changed: 20 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -73,22 +73,6 @@ module Fasthttp {
7373
override string getKind() { result = "URL" }
7474
}
7575

76-
/**
77-
* A function that sends HTTP requests.
78-
*/
79-
class RequestForgerySinkDo extends RequestForgery::Sink {
80-
RequestForgerySinkDo() {
81-
exists(Function f |
82-
f.hasQualifiedName(packagePath(), ["Do", "DoDeadline", "DoTimeout", "DoRedirects"]) and
83-
this = f.getACall().getArgument(0)
84-
)
85-
}
86-
87-
override DataFlow::Node getARequest() { result = this }
88-
89-
override string getKind() { result = "URL" }
90-
}
91-
9276
/**
9377
* A function that create initial connection to a TCP address.
9478
* Following Functions only accept TCP address + Port in their first argument
@@ -192,46 +176,6 @@ module Fasthttp {
192176

193177
override string getKind() { result = "URL" }
194178
}
195-
196-
/**
197-
* A method that sends HTTP requests.
198-
*/
199-
class RequestForgerySinkDo extends RequestForgery::Sink {
200-
RequestForgerySinkDo() {
201-
exists(Method m |
202-
m.hasQualifiedName(packagePath(), "Client",
203-
["Do", "DoDeadline", "DoTimeout", "DoRedirects"]) and
204-
this = m.getACall().getArgument(0)
205-
)
206-
}
207-
208-
override DataFlow::Node getARequest() { result = this }
209-
210-
// Kind can be vary because input is a fasthttp.URI type
211-
override string getKind() { result = "URL" }
212-
}
213-
}
214-
215-
/**
216-
* Provide modeling for fasthttp.PipelineClient Type
217-
*/
218-
module PipelineClient {
219-
/**
220-
* A method that sends HTTP requests.
221-
*/
222-
class RequestForgerySinkDo extends RequestForgery::Sink {
223-
RequestForgerySinkDo() {
224-
exists(Method m |
225-
m.hasQualifiedName(packagePath(), "PipelineClient", ["Do", "DoDeadline", "DoTimeout"]) and
226-
this = m.getACall().getArgument(0)
227-
)
228-
}
229-
230-
override DataFlow::Node getARequest() { result = this }
231-
232-
// Kind can be vary because input is a fasthttp.URI type
233-
override string getKind() { result = "URL" }
234-
}
235179
}
236180

237181
/**
@@ -257,46 +201,6 @@ module Fasthttp {
257201

258202
override string getKind() { result = "URL" }
259203
}
260-
261-
/**
262-
* A method that sends HTTP requests.
263-
*/
264-
class RequestForgerySinkDo extends RequestForgery::Sink {
265-
RequestForgerySinkDo() {
266-
exists(Method m |
267-
m.hasQualifiedName(packagePath(), "HostClient",
268-
["Do", "DoDeadline", "DoTimeout", "DoRedirects"]) and
269-
this = m.getACall().getArgument(0)
270-
)
271-
}
272-
273-
override DataFlow::Node getARequest() { result = this }
274-
275-
// Kind can be vary because input is a fasthttp.URI type
276-
override string getKind() { result = "URL" }
277-
}
278-
}
279-
280-
/**
281-
* Provide modeling for fasthttp.LBClient Type
282-
*/
283-
module LBClient {
284-
/**
285-
* A method that sends HTTP requests.
286-
*/
287-
class RequestForgerySinkDo extends RequestForgery::Sink {
288-
RequestForgerySinkDo() {
289-
exists(Method m |
290-
m.hasQualifiedName(packagePath(), "LBClient", ["Do", "DoDeadline", "DoTimeout"]) and
291-
this = m.getACall().getArgument(0)
292-
)
293-
}
294-
295-
override DataFlow::Node getARequest() { result = this }
296-
297-
// Kind can be vary because input is a fasthttp.URI type
298-
override string getKind() { result = "URL" }
299-
}
300204
}
301205

302206
/**
@@ -354,6 +258,26 @@ module Fasthttp {
354258
)
355259
}
356260
}
261+
262+
/**
263+
* A method that create the URL and Host parts of a `Request` type.
264+
*
265+
* This instance of `Request` type can be used in some functions/methods
266+
* like `func Do(req *Request, resp *Response) error` that will lead to server side request forgery vulnerability.
267+
*/
268+
class RequestForgerySink extends RequestForgery::Sink {
269+
RequestForgerySink() {
270+
exists(Method m |
271+
m.hasQualifiedName(packagePath(), "Request",
272+
["SetRequestURI", "SetRequestURIBytes", "SetURI", "SetHost", "SetHostBytes"]) and
273+
this = m.getACall().getArgument(0)
274+
)
275+
}
276+
277+
override DataFlow::Node getARequest() { result = this }
278+
279+
override string getKind() { result = "URL" }
280+
}
357281
}
358282

359283
/**

‎go/ql/test/library-tests/semmle/go/frameworks/Fasthttp/fasthttp.go‎

Lines changed: 7 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -24,19 +24,7 @@ func fasthttpClient() {
2424
fasthttp.DialTimeout(userInput, 5) // $ SsrfSink=userInput
2525
fasthttp.DialDualStackTimeout(userInput, 5) // $ SsrfSink=userInput
2626

27-
res := &fasthttp.Response{}
28-
req1 := &fasthttp.Request{}
29-
req1.SetHost(source().(string))
30-
sink(req1) // $ hasTaintFlow="req1"
31-
req2 := &fasthttp.Request{}
32-
req2.SetHostBytes(source().([]byte))
33-
sink(req2) // $ hasTaintFlow="req2"
34-
req3 := &fasthttp.Request{}
35-
req3.SetRequestURI(source().(string))
36-
sink(req3) // $ hasTaintFlow="req3"
37-
req4 := &fasthttp.Request{}
38-
req4.SetRequestURIBytes(source().([]byte))
39-
sink(req4) // $ hasTaintFlow="req4"
27+
req := &fasthttp.Request{}
4028

4129
uri1 := fasthttp.AcquireURI()
4230
userInput = "UserControlled.com:80"
@@ -55,20 +43,19 @@ func fasthttpClient() {
5543
uri5 := fasthttp.AcquireURI()
5644
uri5.Parse(source().([]byte), source().([]byte))
5745
sink(uri5) // $ hasTaintFlow="uri5"
58-
req := &fasthttp.Request{}
59-
uri6 := fasthttp.AcquireURI()
60-
req.SetURI(uri6)
6146

6247
resByte := make([]byte, 1000)
6348
userInput = "http://127.0.0.1:8909"
49+
userInputBytes := []byte("http://127.0.0.1:8909")
50+
req.SetURI(uri5) // $ SsrfSink=uri5
51+
req.SetHost(userInput) // $ SsrfSink=userInput
52+
req.SetHostBytes(userInputBytes) // $ SsrfSink=userInputBytes
53+
req.SetRequestURI(userInput) // $ SsrfSink=userInput
54+
req.SetRequestURIBytes(userInputBytes) // $ SsrfSink=userInputBytes
6455
fasthttp.Get(resByte, userInput) // $ SsrfSink=userInput
6556
fasthttp.GetDeadline(resByte, userInput, time.Time{}) // $ SsrfSink=userInput
6657
fasthttp.GetTimeout(resByte, userInput, 5) // $ SsrfSink=userInput
6758
fasthttp.Post(resByte, userInput, nil) // $ SsrfSink=userInput
68-
fasthttp.Do(req, res) // $ SsrfSink=req
69-
fasthttp.DoRedirects(req, res, 2) // $ SsrfSink=req
70-
fasthttp.DoDeadline(req, res, time.Time{}) // $ SsrfSink=req
71-
fasthttp.DoTimeout(req, res, 5) // $ SsrfSink=req
7259

7360
hostClient := &fasthttp.HostClient{
7461
Addr: "localhost:8080",
@@ -77,31 +64,15 @@ func fasthttpClient() {
7764
hostClient.GetDeadline(resByte, userInput, time.Time{}) // $ SsrfSink=userInput
7865
hostClient.GetTimeout(resByte, userInput, 5) // $ SsrfSink=userInput
7966
hostClient.Post(resByte, userInput, nil) // $ SsrfSink=userInput
80-
hostClient.Do(req, res) // $ SsrfSink=req
81-
hostClient.DoDeadline(req, res, time.Time{}) // $ SsrfSink=req
82-
hostClient.DoRedirects(req, res, 2) // $ SsrfSink=req
83-
hostClient.DoTimeout(req, res, 5) // $ SsrfSink=req
8467

8568
var lbclient fasthttp.LBClient
8669
lbclient.Clients = append(lbclient.Clients, hostClient)
87-
lbclient.Do(req, res) // $ SsrfSink=req
88-
lbclient.DoDeadline(req, res, time.Time{}) // $ SsrfSink=req
89-
lbclient.DoTimeout(req, res, 5) // $ SsrfSink=req
9070

9171
client := fasthttp.Client{}
9272
client.Get(resByte, userInput) // $ SsrfSink=userInput
9373
client.GetDeadline(resByte, userInput, time.Time{}) // $ SsrfSink=userInput
9474
client.GetTimeout(resByte, userInput, 5) // $ SsrfSink=userInput
9575
client.Post(resByte, userInput, nil) // $ SsrfSink=userInput
96-
client.Do(req, res) // $ SsrfSink=req
97-
client.DoDeadline(req, res, time.Time{}) // $ SsrfSink=req
98-
client.DoRedirects(req, res, 2) // $ SsrfSink=req
99-
client.DoTimeout(req, res, 5) // $ SsrfSink=req
100-
101-
pipelineClient := fasthttp.PipelineClient{}
102-
pipelineClient.Do(req, res) // $ SsrfSink=req
103-
pipelineClient.DoDeadline(req, res, time.Time{}) // $ SsrfSink=req
104-
pipelineClient.DoTimeout(req, res, 5) // $ SsrfSink=req
10576

10677
tcpDialer := fasthttp.TCPDialer{}
10778
userInput = "127.0.0.1:8909"

0 commit comments

Comments
 (0)