From 830c3fce2a44be2b7159c54bc6f61c336b6214e4 Mon Sep 17 00:00:00 2001 From: Sauyon Lee Date: Fri, 27 Mar 2020 04:23:43 -0700 Subject: [PATCH] RequestForgery: Add tests --- .../Security/CWE-918/RequestForgery.expected | 38 +++++++++++++++++ .../Security/CWE-918/RequestForgery.go | 18 ++++++++ .../Security/CWE-918/RequestForgery.qlref | 1 + .../Security/CWE-918/RequestForgeryGood.go | 25 +++++++++++ ql/test/query-tests/Security/CWE-918/tst.go | 42 +++++++++++++++++++ ql/test/query-tests/Security/CWE-918/util.go | 5 +++ 6 files changed, 129 insertions(+) create mode 100644 ql/test/query-tests/Security/CWE-918/RequestForgery.expected create mode 100644 ql/test/query-tests/Security/CWE-918/RequestForgery.go create mode 100644 ql/test/query-tests/Security/CWE-918/RequestForgery.qlref create mode 100644 ql/test/query-tests/Security/CWE-918/RequestForgeryGood.go create mode 100644 ql/test/query-tests/Security/CWE-918/tst.go create mode 100644 ql/test/query-tests/Security/CWE-918/util.go diff --git a/ql/test/query-tests/Security/CWE-918/RequestForgery.expected b/ql/test/query-tests/Security/CWE-918/RequestForgery.expected new file mode 100644 index 00000000000..5b8fb08dce1 --- /dev/null +++ b/ql/test/query-tests/Security/CWE-918/RequestForgery.expected @@ -0,0 +1,38 @@ +edges +| RequestForgery.go:8:12:8:34 | call to FormValue : string | RequestForgery.go:11:24:11:65 | ...+... | +| tst.go:10:13:10:35 | call to FormValue : string | tst.go:14:11:14:17 | tainted | +| tst.go:10:13:10:35 | call to FormValue : string | tst.go:18:12:18:18 | tainted | +| tst.go:10:13:10:35 | call to FormValue : string | tst.go:21:34:21:40 | tainted | +| tst.go:10:13:10:35 | call to FormValue : string | tst.go:24:66:24:72 | tainted | +| tst.go:10:13:10:35 | call to FormValue : string | tst.go:27:11:27:29 | ...+... | +| tst.go:10:13:10:35 | call to FormValue : string | tst.go:29:11:29:40 | ...+... | +| tst.go:10:13:10:35 | call to FormValue : string | tst.go:36:2:36:2 | implicit dereference : URL | +| tst.go:10:13:10:35 | call to FormValue : string | tst.go:37:11:37:20 | call to String | +| tst.go:35:2:35:2 | definition of u [pointer] : URL | tst.go:36:2:36:2 | u [pointer] : URL | +| tst.go:36:2:36:2 | implicit dereference : URL | tst.go:35:2:35:2 | definition of u [pointer] : URL | +| tst.go:36:2:36:2 | implicit dereference : URL | tst.go:36:2:36:2 | implicit dereference : URL | +| tst.go:36:2:36:2 | implicit dereference : URL | tst.go:37:11:37:20 | call to String | +| tst.go:36:2:36:2 | u [pointer] : URL | tst.go:36:2:36:2 | implicit dereference : URL | +nodes +| RequestForgery.go:8:12:8:34 | call to FormValue : string | semmle.label | call to FormValue : string | +| RequestForgery.go:11:24:11:65 | ...+... | semmle.label | ...+... | +| tst.go:10:13:10:35 | call to FormValue : string | semmle.label | call to FormValue : string | +| tst.go:14:11:14:17 | tainted | semmle.label | tainted | +| tst.go:18:12:18:18 | tainted | semmle.label | tainted | +| tst.go:21:34:21:40 | tainted | semmle.label | tainted | +| tst.go:24:66:24:72 | tainted | semmle.label | tainted | +| tst.go:27:11:27:29 | ...+... | semmle.label | ...+... | +| tst.go:29:11:29:40 | ...+... | semmle.label | ...+... | +| tst.go:35:2:35:2 | definition of u [pointer] : URL | semmle.label | definition of u [pointer] : URL | +| tst.go:36:2:36:2 | implicit dereference : URL | semmle.label | implicit dereference : URL | +| tst.go:36:2:36:2 | u [pointer] : URL | semmle.label | u [pointer] : URL | +| tst.go:37:11:37:20 | call to String | semmle.label | call to String | +#select +| RequestForgery.go:11:15:11:66 | call to Get | RequestForgery.go:8:12:8:34 | call to FormValue : string | RequestForgery.go:11:24:11:65 | ...+... | The $@ of this request depends on $@. | RequestForgery.go:11:24:11:65 | ...+... | URL | RequestForgery.go:8:12:8:34 | call to FormValue : string | a user-provided value | +| tst.go:14:2:14:18 | call to Get | tst.go:10:13:10:35 | call to FormValue : string | tst.go:14:11:14:17 | tainted | The $@ of this request depends on $@. | tst.go:14:11:14:17 | tainted | URL | tst.go:10:13:10:35 | call to FormValue : string | a user-provided value | +| tst.go:18:2:18:38 | call to Post | tst.go:10:13:10:35 | call to FormValue : string | tst.go:18:12:18:18 | tainted | The $@ of this request depends on $@. | tst.go:18:12:18:18 | tainted | URL | tst.go:10:13:10:35 | call to FormValue : string | a user-provided value | +| tst.go:22:2:22:14 | call to Do | tst.go:10:13:10:35 | call to FormValue : string | tst.go:21:34:21:40 | tainted | The $@ of this request depends on $@. | tst.go:21:34:21:40 | tainted | URL | tst.go:10:13:10:35 | call to FormValue : string | a user-provided value | +| tst.go:25:2:25:14 | call to Do | tst.go:10:13:10:35 | call to FormValue : string | tst.go:24:66:24:72 | tainted | The $@ of this request depends on $@. | tst.go:24:66:24:72 | tainted | URL | tst.go:10:13:10:35 | call to FormValue : string | a user-provided value | +| tst.go:27:2:27:30 | call to Get | tst.go:10:13:10:35 | call to FormValue : string | tst.go:27:11:27:29 | ...+... | The $@ of this request depends on $@. | tst.go:27:11:27:29 | ...+... | URL | tst.go:10:13:10:35 | call to FormValue : string | a user-provided value | +| tst.go:29:2:29:41 | call to Get | tst.go:10:13:10:35 | call to FormValue : string | tst.go:29:11:29:40 | ...+... | The $@ of this request depends on $@. | tst.go:29:11:29:40 | ...+... | URL | tst.go:10:13:10:35 | call to FormValue : string | a user-provided value | +| tst.go:37:2:37:21 | call to Get | tst.go:10:13:10:35 | call to FormValue : string | tst.go:37:11:37:20 | call to String | The $@ of this request depends on $@. | tst.go:37:11:37:20 | call to String | URL | tst.go:10:13:10:35 | call to FormValue : string | a user-provided value | diff --git a/ql/test/query-tests/Security/CWE-918/RequestForgery.go b/ql/test/query-tests/Security/CWE-918/RequestForgery.go new file mode 100644 index 00000000000..c88e9dc5f74 --- /dev/null +++ b/ql/test/query-tests/Security/CWE-918/RequestForgery.go @@ -0,0 +1,18 @@ +package main + +import ( + "net/http" +) + +func handler(w http.ResponseWriter, req *http.Request) { + target := req.FormValue("target") + + // BAD: `target` is controlled by the attacker + resp, err := http.Get("https://" + target + ".example.com/data/") + if err != nil { + // error handling + } + + // process request response + use(resp) +} diff --git a/ql/test/query-tests/Security/CWE-918/RequestForgery.qlref b/ql/test/query-tests/Security/CWE-918/RequestForgery.qlref new file mode 100644 index 00000000000..fcb4e41daf8 --- /dev/null +++ b/ql/test/query-tests/Security/CWE-918/RequestForgery.qlref @@ -0,0 +1 @@ +Security/CWE-918/RequestForgery.ql diff --git a/ql/test/query-tests/Security/CWE-918/RequestForgeryGood.go b/ql/test/query-tests/Security/CWE-918/RequestForgeryGood.go new file mode 100644 index 00000000000..92f7b5fc1d5 --- /dev/null +++ b/ql/test/query-tests/Security/CWE-918/RequestForgeryGood.go @@ -0,0 +1,25 @@ +package main + +import ( + "net/http" +) + +func handler1(w http.ResponseWriter, req *http.Request) { + target := req.FormValue("target") + + var subdomain string + if target == "EU" { + subdomain = "europe" + } else { + subdomain = "world" + } + + // GOOD: `subdomain` is controlled by the server + resp, err := http.Get("https://" + subdomain + ".example.com/data/") + if err != nil { + // error handling + } + + // process request response + use(resp) +} diff --git a/ql/test/query-tests/Security/CWE-918/tst.go b/ql/test/query-tests/Security/CWE-918/tst.go new file mode 100644 index 00000000000..0e04429580c --- /dev/null +++ b/ql/test/query-tests/Security/CWE-918/tst.go @@ -0,0 +1,42 @@ +package main + +import ( + "context" + "net/http" + "net/url" +) + +func handler2(w http.ResponseWriter, req *http.Request) { + tainted := req.FormValue("target") + + http.Get("example.com") // OK + + http.Get(tainted) // Not OK + + http.Head(tainted) // OK + + http.Post(tainted, "text/basic", nil) // Not OK + + client := &http.Client{} + rq, _ := http.NewRequest("GET", tainted, nil) + client.Do(rq) // Not OK + + rq, _ = http.NewRequestWithContext(context.Background(), "GET", tainted, nil) + client.Do(rq) // Not OK + + http.Get("http://" + tainted) // Not OK + + http.Get("http://example.com" + tainted) // Not OK + + http.Get("http://example.com/" + tainted) // OK + + http.Get("http://example.com/?" + tainted) // OK + + u, _ := url.Parse("http://example.com/relative-path") + u.Host = tainted + http.Get(u.String()) // Not OK +} + +func main() { + +} diff --git a/ql/test/query-tests/Security/CWE-918/util.go b/ql/test/query-tests/Security/CWE-918/util.go new file mode 100644 index 00000000000..5655c4f0cff --- /dev/null +++ b/ql/test/query-tests/Security/CWE-918/util.go @@ -0,0 +1,5 @@ +package main + +func use(vars ...interface{}) { + +}