Skip to content

Commit 88e75a6

Browse files
am0o0am0o0
committed
add flow summary instead of additional flow steps
1 parent 23f7f9a commit 88e75a6

File tree

6 files changed

+83
-152
lines changed

6 files changed

+83
-152
lines changed

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
extensions:
2+
- addsTo:
3+
pack: codeql/go-all
4+
extensible: summaryModel
5+
data:
6+
- ["github.com/valyala/fasthttp", "URI", False, "SetHost", "", "", "Argument[0]", "Argument[-1]", "taint", "manual"]
7+
- ["github.com/valyala/fasthttp", "URI", False, "SetHostBytes", "", "", "Argument[0]", "Argument[-1]", "taint", "manual"]
8+
- ["github.com/valyala/fasthttp", "URI", False, "Update", "", "", "Argument[0]", "Argument[-1]", "taint", "manual"]
9+
- ["github.com/valyala/fasthttp", "URI", False, "UpdateBytes", "", "", "Argument[0]", "Argument[-1]", "taint", "manual"]
10+
- ["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"]

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

Lines changed: 0 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,6 @@ module Fasthttp {
1616
/** Gets the path for the root package of fasthttp. */
1717
string packagePath() { result = package(v1modulePath(), "") }
1818

19-
/**
20-
* A class when you are using Fasthttp related queries to fully supports additional steps
21-
*/
22-
bindingset[this]
23-
abstract class AdditionalStep extends string {
24-
/**
25-
* Holds if `pred` to `succ` is an additional taint-propagating step for this query.
26-
*/
27-
abstract predicate hasTaintStep(DataFlow::Node pred, DataFlow::Node succ);
28-
}
29-
3019
/**
3120
* Provide models for sanitizer/Dangerous Functions of fasthttp
3221
*/
@@ -125,32 +114,6 @@ module Fasthttp {
125114
* Provide modeling for fasthttp.URI Type
126115
*/
127116
module URI {
128-
/**
129-
* The additioanl steps that can be used in fasthttp framework.
130-
* Fasthttp has its own uri creating/manipulation methods and these methods usually are used in code.
131-
* Pred can be an user controlled value like any potential part of URL and succ is the URI instance.
132-
* So if we called a method like `URIInstance.SetHost(pred)` then the URIInstance is succ.
133-
*/
134-
class UriAdditionalStep extends AdditionalStep {
135-
UriAdditionalStep() { this = "URI additional steps" }
136-
137-
override predicate hasTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
138-
exists(DataFlow::MethodCallNode m, DataFlow::Variable frn |
139-
(
140-
m.getTarget()
141-
.hasQualifiedName(packagePath(), "URI",
142-
["SetHost", "SetHostBytes", "Update", "UpdateBytes"]) and
143-
pred = m.getArgument(0)
144-
or
145-
m.getTarget().hasQualifiedName(packagePath(), "URI", "Parse") and
146-
pred = m.getArgument([0, 1])
147-
) and
148-
frn.getARead() = m.getReceiver() and
149-
succ = frn.getARead()
150-
)
151-
}
152-
}
153-
154117
/**
155118
* The methods as Remote user controllable source which are part of the incoming URL
156119
*/
@@ -384,30 +347,6 @@ module Fasthttp {
384347
* Provide modeling for fasthttp.Request Type
385348
*/
386349
module Request {
387-
/**
388-
* The additioanl steps that can be used in fasthttp framework.
389-
* Pred can be an user controlled value like any potential part of URL and succ is the URI instance.
390-
* So if we called a method like `RequestInstance.SetHost(pred)` then the RequestInstance is succ.
391-
* for SetURI the argument type is fasthttp.URI which is already modeled, look at URI module.
392-
*/
393-
class RequestAdditionalStep extends AdditionalStep {
394-
RequestAdditionalStep() { this = "Request additional steps" }
395-
396-
override predicate hasTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
397-
exists(DataFlow::MethodCallNode m, DataFlow::Variable frn |
398-
m.getTarget()
399-
.hasQualifiedName(packagePath(), "Request",
400-
[
401-
"SetRequestURI", "SetRequestURIBytes", "SetURI", "String", "SetHost",
402-
"SetHostBytes"
403-
]) and
404-
pred = m.getArgument(0) and
405-
frn.getARead() = m.getReceiver() and
406-
succ = frn.getARead()
407-
)
408-
}
409-
}
410-
411350
/**
412351
* The methods as Remote user controllable source which can be many part of request
413352
*/

‎go/ql/src/experimental/CWE-918/SSRF.qll

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -175,30 +175,4 @@ module ServerSideRequestForgery {
175175
* of the error binding exists, and the tag to check is one of "alpha", "alphanum", "alphaunicode", "alphanumunicode", "number", "numeric".
176176
*/
177177
class ValidatorAsSanitizer extends Sanitizer, ValidatorVarCheckBarrier { }
178-
179-
/**
180-
* A additional step that can be used mostly for request forgery related queries
181-
*/
182-
bindingset[this]
183-
abstract class AdditionalStep extends string {
184-
/**
185-
* Holds if `pred` to `succ` is an additional taint-propagating step for this query.
186-
*/
187-
abstract predicate hasTaintStep(DataFlow::Node pred, DataFlow::Node succ);
188-
}
189-
190-
/**
191-
* An additional step for Fasthttp framework uri and request instances.
192-
*
193-
* These steps can help to track the user provided URI to a dangerous SSRF sink.
194-
*/
195-
class FasthttpAdditionalStep extends AdditionalStep {
196-
FasthttpAdditionalStep() { this = "FastHTtp additional steps" }
197-
198-
override predicate hasTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
199-
any(Fasthttp::Request::RequestAdditionalStep r).hasTaintStep(pred, succ)
200-
or
201-
any(Fasthttp::URI::UriAdditionalStep r).hasTaintStep(pred, succ)
202-
}
203-
}
204178
}

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +0,0 @@
1-
testFailures
2-
failures

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

Lines changed: 37 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,46 +1,48 @@
11
import go
2-
import TestUtilities.InlineExpectationsTest
2+
import TestUtilities.InlineFlowTest
3+
import semmle.go.security.RequestForgeryCustomizations
34

4-
module FasthttpTest implements TestSig {
5-
string getARelevantTag() { result = ["UriSucc", "UriPred", "ReqSucc", "ReqPred"] }
5+
module Config implements DataFlow::ConfigSig {
6+
predicate isSource(DataFlow::Node source) {
7+
exists(DataFlow::MethodCallNode m |
8+
m.getTarget()
9+
.hasQualifiedName("github.com/valyala/fasthttp", "URI",
10+
["SetHost", "SetHostBytes", "Update", "UpdateBytes"]) and
11+
source = m.getArgument(0)
12+
or
13+
m.getTarget().hasQualifiedName("github.com/valyala/fasthttp", "URI", "Parse") and
14+
source = m.getArgument([0, 1])
15+
)
16+
or
17+
exists(DataFlow::MethodCallNode m |
18+
m.getTarget()
19+
.hasQualifiedName("github.com/valyala/fasthttp", "Request",
20+
["SetRequestURI", "SetRequestURIBytes", "SetURI", "String", "SetHost", "SetHostBytes"]) and
21+
source = m.getArgument(0)
22+
)
23+
}
624

7-
predicate hasActualResult(Location location, string element, string tag, string value) {
8-
exists(Fasthttp::Request::RequestAdditionalStep q, DataFlow::Node succ, DataFlow::Node pred |
9-
q.hasTaintStep(pred, succ)
10-
|
25+
predicate isSink(DataFlow::Node source) {
26+
exists(DataFlow::MethodCallNode m, DataFlow::Variable frn |
1127
(
12-
pred.hasLocationInfo(location.getFile().getAbsolutePath(), location.getStartLine(),
13-
location.getStartColumn(), location.getEndLine(), location.getEndColumn()) and
14-
element = pred.toString() and
15-
value = pred.toString() and
16-
tag = "ReqPred"
28+
m.getTarget()
29+
.hasQualifiedName("github.com/valyala/fasthttp", "URI",
30+
["SetHost", "SetHostBytes", "Update", "UpdateBytes"])
1731
or
18-
succ.hasLocationInfo(location.getFile().getAbsolutePath(), location.getStartLine(),
19-
location.getStartColumn(), location.getEndLine(), location.getEndColumn()) and
20-
element = succ.toString() and
21-
value = succ.toString() and
22-
tag = "ReqSucc"
23-
)
32+
m.getTarget().hasQualifiedName("github.com/valyala/fasthttp", "URI", "Parse")
33+
) and
34+
frn.getARead() = m.getReceiver() and
35+
source = frn.getARead()
2436
)
2537
or
26-
exists(Fasthttp::URI::UriAdditionalStep q, DataFlow::Node succ, DataFlow::Node pred |
27-
q.hasTaintStep(pred, succ)
28-
|
29-
(
30-
pred.hasLocationInfo(location.getFile().getAbsolutePath(), location.getStartLine(),
31-
location.getStartColumn(), location.getEndLine(), location.getEndColumn()) and
32-
element = pred.toString() and
33-
value = pred.toString() and
34-
tag = "UriPred"
35-
or
36-
succ.hasLocationInfo(location.getFile().getAbsolutePath(), location.getStartLine(),
37-
location.getStartColumn(), location.getEndLine(), location.getEndColumn()) and
38-
element = succ.toString() and
39-
value = succ.toString() and
40-
tag = "UriSucc"
41-
)
38+
exists(DataFlow::MethodCallNode m, DataFlow::Variable frn |
39+
m.getTarget()
40+
.hasQualifiedName("github.com/valyala/fasthttp", "Request",
41+
["SetRequestURI", "SetRequestURIBytes", "SetURI", "String", "SetHost", "SetHostBytes"]) and
42+
frn.getARead() = m.getReceiver() and
43+
source = frn.getARead()
4244
)
4345
}
4446
}
4547

46-
import MakeTest<FasthttpTest>
48+
import TaintFlowTest<Config>

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

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -20,33 +20,33 @@ func fasthttpClient() {
2020

2121
res := &fasthttp.Response{}
2222
req := &fasthttp.Request{}
23-
req.SetHost(userInput) // $ ReqSucc=req ReqPred=userInput
24-
req.SetHostBytes(userInputByte) // $ ReqSucc=req ReqPred=userInputByte
25-
req.SetRequestURI(userInput) // $ ReqSucc=req ReqPred=userInput
26-
req.SetRequestURIBytes(userInputByte) // $ ReqSucc=req ReqPred=userInputByte
23+
req.SetHost(userInput) // $ hasTaintFlow="req" ReqPred=userInput
24+
req.SetHostBytes(userInputByte) // $ hasTaintFlow="req" ReqPred=userInputByte
25+
req.SetRequestURI(userInput) // $ hasTaintFlow="req" ReqPred=userInput
26+
req.SetRequestURIBytes(userInputByte) // $ hasTaintFlow="req" ReqPred=userInputByte
2727

2828
uri := fasthttp.AcquireURI()
2929
userInput = "UserControlled.com:80"
3030
userInputByte = []byte("UserControlled.com:80")
31-
uri.SetHost(userInput) // $ UriPred=userInput UriSucc=uri
32-
uri.SetHostBytes(userInputByte) // $ UriPred=userInputByte UriSucc=uri
31+
uri.SetHost(userInput) // $ hasTaintFlow="uri"
32+
uri.SetHostBytes(userInputByte) // $ hasTaintFlow="uri"
3333
userInput = "http://UserControlled.com"
3434
userInputByte = []byte("http://UserControlled.com")
35-
uri.Update(userInput) // $ UriPred=userInput UriSucc=uri
36-
uri.UpdateBytes(userInputByte) // $ UriPred=userInputByte UriSucc=uri
37-
uri.Parse(userInputByte, userInputByte) // $ UriPred=userInputByte UriPred=userInputByte UriSucc=uri
38-
req.SetURI(uri) // $ ReqSucc=req ReqPred=uri UriSucc=uri
35+
uri.Update(userInput) // $ hasTaintFlow="uri"
36+
uri.UpdateBytes(userInputByte) // $ hasTaintFlow="uri"
37+
uri.Parse(userInputByte, userInputByte) // $ hasTaintFlow="uri"
38+
req.SetURI(uri) // $ hasTaintFlow="uri" hasTaintFlow="req"
3939

4040
resByte := make([]byte, 1000)
4141
userInput = "http://127.0.0.1:8909"
4242
fasthttp.Get(resByte, userInput) // $ SsrfSink=userInput
4343
fasthttp.GetDeadline(resByte, userInput, time.Time{}) // $ SsrfSink=userInput
4444
fasthttp.GetTimeout(resByte, userInput, 5) // $ SsrfSink=userInput
4545
fasthttp.Post(resByte, userInput, nil) // $ SsrfSink=userInput
46-
fasthttp.Do(req, res) // $ ReqSucc=req SsrfSink=req
47-
fasthttp.DoRedirects(req, res, 2) // $ ReqSucc=req SsrfSink=req
48-
fasthttp.DoDeadline(req, res, time.Time{}) // $ ReqSucc=req SsrfSink=req
49-
fasthttp.DoTimeout(req, res, 5) // $ ReqSucc=req SsrfSink=req
46+
fasthttp.Do(req, res) // $ hasTaintFlow="req" SsrfSink=req
47+
fasthttp.DoRedirects(req, res, 2) // $ hasTaintFlow="req" SsrfSink=req
48+
fasthttp.DoDeadline(req, res, time.Time{}) // $ hasTaintFlow="req" SsrfSink=req
49+
fasthttp.DoTimeout(req, res, 5) // $ hasTaintFlow="req" SsrfSink=req
5050

5151
hostClient := &fasthttp.HostClient{
5252
Addr: "localhost:8080",
@@ -55,31 +55,31 @@ func fasthttpClient() {
5555
hostClient.GetDeadline(resByte, userInput, time.Time{}) // $ SsrfSink=userInput
5656
hostClient.GetTimeout(resByte, userInput, 5) // $ SsrfSink=userInput
5757
hostClient.Post(resByte, userInput, nil) // $ SsrfSink=userInput
58-
hostClient.Do(req, res) // $ ReqSucc=req SsrfSink=req
59-
hostClient.DoDeadline(req, res, time.Time{}) // $ ReqSucc=req SsrfSink=req
60-
hostClient.DoRedirects(req, res, 2) // $ ReqSucc=req SsrfSink=req
61-
hostClient.DoTimeout(req, res, 5) // $ ReqSucc=req SsrfSink=req
58+
hostClient.Do(req, res) // $ hasTaintFlow="req" SsrfSink=req
59+
hostClient.DoDeadline(req, res, time.Time{}) // $ hasTaintFlow="req" SsrfSink=req
60+
hostClient.DoRedirects(req, res, 2) // $ hasTaintFlow="req" SsrfSink=req
61+
hostClient.DoTimeout(req, res, 5) // $ hasTaintFlow="req" SsrfSink=req
6262

6363
var lbclient fasthttp.LBClient
6464
lbclient.Clients = append(lbclient.Clients, hostClient)
65-
lbclient.Do(req, res) // $ ReqSucc=req SsrfSink=req
66-
lbclient.DoDeadline(req, res, time.Time{}) // $ ReqSucc=req SsrfSink=req
67-
lbclient.DoTimeout(req, res, 5) // $ ReqSucc=req SsrfSink=req
65+
lbclient.Do(req, res) // $ hasTaintFlow="req" SsrfSink=req
66+
lbclient.DoDeadline(req, res, time.Time{}) // $ hasTaintFlow="req" SsrfSink=req
67+
lbclient.DoTimeout(req, res, 5) // $ hasTaintFlow="req" SsrfSink=req
6868

6969
client := fasthttp.Client{}
7070
client.Get(resByte, userInput) // $ SsrfSink=userInput
7171
client.GetDeadline(resByte, userInput, time.Time{}) // $ SsrfSink=userInput
7272
client.GetTimeout(resByte, userInput, 5) // $ SsrfSink=userInput
7373
client.Post(resByte, userInput, nil) // $ SsrfSink=userInput
74-
client.Do(req, res) // $ ReqSucc=req SsrfSink=req SsrfSink=req
75-
client.DoDeadline(req, res, time.Time{}) // $ ReqSucc=req SsrfSink=req SsrfSink=req
76-
client.DoRedirects(req, res, 2) // $ ReqSucc=req SsrfSink=req SsrfSink=req
77-
client.DoTimeout(req, res, 5) // $ ReqSucc=req SsrfSink=req SsrfSink=req
74+
client.Do(req, res) // $ hasTaintFlow="req" SsrfSink=req SsrfSink=req
75+
client.DoDeadline(req, res, time.Time{}) // $ hasTaintFlow="req" SsrfSink=req SsrfSink=req
76+
client.DoRedirects(req, res, 2) // $ hasTaintFlow="req" SsrfSink=req SsrfSink=req
77+
client.DoTimeout(req, res, 5) // $ hasTaintFlow="req" SsrfSink=req SsrfSink=req
7878

7979
pipelineClient := fasthttp.PipelineClient{}
80-
pipelineClient.Do(req, res) // $ ReqSucc=req SsrfSink=req SsrfSink=req
81-
pipelineClient.DoDeadline(req, res, time.Time{}) // $ ReqSucc=req SsrfSink=req SsrfSink=req
82-
pipelineClient.DoTimeout(req, res, 5) // $ ReqSucc=req SsrfSink=req SsrfSink=req
80+
pipelineClient.Do(req, res) // $ hasTaintFlow="req" SsrfSink=req SsrfSink=req
81+
pipelineClient.DoDeadline(req, res, time.Time{}) // $ hasTaintFlow="req" SsrfSink=req SsrfSink=req
82+
pipelineClient.DoTimeout(req, res, 5) // $ hasTaintFlow="req" SsrfSink=req SsrfSink=req
8383

8484
tcpDialer := fasthttp.TCPDialer{}
8585
userInput = "127.0.0.1:8909"

0 commit comments

Comments
 (0)