diff --git a/go/ql/lib/change-notes/2025-09-30-fewer-safe-urls.md b/go/ql/lib/change-notes/2025-09-30-fewer-safe-urls.md new file mode 100644 index 000000000000..5eeee51c4a3c --- /dev/null +++ b/go/ql/lib/change-notes/2025-09-30-fewer-safe-urls.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* `go/unvalidated-url-redirection` and `go/request-forgery` have a shared notion of a safe URL, which is known to not be malicious. Some URLs which were incorrectly considered safe are now correctly considered unsafe. This may lead to more alerts for those two queries. diff --git a/go/ql/lib/semmle/go/security/OpenUrlRedirectCustomizations.qll b/go/ql/lib/semmle/go/security/OpenUrlRedirectCustomizations.qll index 870edeee9621..c278bdf58c5d 100644 --- a/go/ql/lib/semmle/go/security/OpenUrlRedirectCustomizations.qll +++ b/go/ql/lib/semmle/go/security/OpenUrlRedirectCustomizations.qll @@ -6,7 +6,7 @@ import go import UrlConcatenation -import SafeUrlFlowCustomizations +private import SafeUrlFlowCustomizations import semmle.go.dataflow.barrierguardutil.RedirectCheckBarrierGuard import semmle.go.dataflow.barrierguardutil.RegexpCheck import semmle.go.dataflow.barrierguardutil.UrlCheck @@ -121,21 +121,6 @@ module OpenUrlRedirect { /** A sink for an open redirect, considered as a sink for safe URL flow. */ private class SafeUrlSink extends SafeUrlFlow::Sink instanceof OpenUrlRedirect::Sink { } -/** - * A read of a field considered unsafe to redirect to, considered as a sanitizer for a safe - * URL. - */ -private class UnsafeFieldReadSanitizer extends SafeUrlFlow::SanitizerEdge { - UnsafeFieldReadSanitizer() { - exists(DataFlow::FieldReadNode frn, string name | - name = ["User", "RawQuery", "Fragment"] and - frn.getField().hasQualifiedName("net/url", "URL") - | - this = frn.getBase() - ) - } -} - /** * Reinstate the usual field propagation rules for fields, which the OpenURLRedirect * query usually excludes, for fields of `Params` other than `Params.Fixed`. diff --git a/go/ql/lib/semmle/go/security/RequestForgeryCustomizations.qll b/go/ql/lib/semmle/go/security/RequestForgeryCustomizations.qll index 2449ffe488ca..1298785b726c 100644 --- a/go/ql/lib/semmle/go/security/RequestForgeryCustomizations.qll +++ b/go/ql/lib/semmle/go/security/RequestForgeryCustomizations.qll @@ -4,7 +4,7 @@ import go import UrlConcatenation -import SafeUrlFlowCustomizations +private import SafeUrlFlowCustomizations import semmle.go.dataflow.barrierguardutil.RedirectCheckBarrierGuard import semmle.go.dataflow.barrierguardutil.RegexpCheck import semmle.go.dataflow.barrierguardutil.UrlCheck @@ -118,18 +118,3 @@ module RequestForgery { /** A sink for request forgery, considered as a sink for safe URL flow. */ private class SafeUrlSink extends SafeUrlFlow::Sink instanceof RequestForgery::Sink { } - -/** - * A read of a field considered unsafe for request forgery, considered as a sanitizer for a safe - * URL. - */ -private class UnsafeFieldReadSanitizer extends SafeUrlFlow::SanitizerEdge { - UnsafeFieldReadSanitizer() { - exists(DataFlow::FieldReadNode frn, string name | - (name = "RawQuery" or name = "Fragment" or name = "User") and - frn.getField().hasQualifiedName("net/url", "URL") - | - this = frn.getBase() - ) - } -} diff --git a/go/ql/lib/semmle/go/security/SafeUrlFlow.qll b/go/ql/lib/semmle/go/security/SafeUrlFlow.qll index 77b7aeda591b..1fc39072dfbf 100644 --- a/go/ql/lib/semmle/go/security/SafeUrlFlow.qll +++ b/go/ql/lib/semmle/go/security/SafeUrlFlow.qll @@ -30,8 +30,10 @@ module SafeUrlFlow { predicate isBarrierOut(DataFlow::Node node) { // block propagation of this safe value when its host is overwritten - exists(Write w, Field f | f.hasQualifiedName("net/url", "URL", "Host") | - w.writesField(node.getASuccessor(), f, _) + exists(Write w, DataFlow::Node b, Field f | + f.hasQualifiedName("net/url", "URL", "Host") and + b = node.getASuccessor() and + w.writesField(b, f, _) ) or node instanceof SanitizerEdge diff --git a/go/ql/lib/semmle/go/security/SafeUrlFlowCustomizations.qll b/go/ql/lib/semmle/go/security/SafeUrlFlowCustomizations.qll index 5f0572db11ef..8acd88fb26d8 100644 --- a/go/ql/lib/semmle/go/security/SafeUrlFlowCustomizations.qll +++ b/go/ql/lib/semmle/go/security/SafeUrlFlowCustomizations.qll @@ -40,4 +40,19 @@ module SafeUrlFlow { private class StringSlicingEdge extends SanitizerEdge { StringSlicingEdge() { this = any(DataFlow::SliceNode sn) } } + + /** + * A read of a field considered unsafe to redirect to, considered as a sanitizer for a safe + * URL. + */ + private class UnsafeFieldReadSanitizer extends SanitizerEdge { + UnsafeFieldReadSanitizer() { + exists(DataFlow::FieldReadNode frn, string name | + name = ["Fragment", "RawQuery", "User"] and + frn.getField().hasQualifiedName("net/url", "URL", name) + | + this = frn.getBase() + ) + } + } } diff --git a/go/ql/test/library-tests/semmle/go/security/SafeUrlFlow/SafeUrlFlow.expected b/go/ql/test/library-tests/semmle/go/security/SafeUrlFlow/SafeUrlFlow.expected new file mode 100644 index 000000000000..79e0c9145fe9 --- /dev/null +++ b/go/ql/test/library-tests/semmle/go/security/SafeUrlFlow/SafeUrlFlow.expected @@ -0,0 +1,115 @@ +#select +| SafeUrlFlow.go:11:24:11:50 | ...+... | SafeUrlFlow.go:10:14:10:21 | selection of Host | SafeUrlFlow.go:11:24:11:50 | ...+... | A safe URL flows here from $@. | SafeUrlFlow.go:10:14:10:21 | selection of Host | here | +| SafeUrlFlow.go:14:29:14:44 | call to String | SafeUrlFlow.go:13:13:13:19 | selection of URL | SafeUrlFlow.go:14:29:14:44 | call to String | A safe URL flows here from $@. | SafeUrlFlow.go:13:13:13:19 | selection of URL | here | +| SafeUrlFlow.go:18:11:18:28 | call to String | SafeUrlFlow.go:10:14:10:21 | selection of Host | SafeUrlFlow.go:18:11:18:28 | call to String | A safe URL flows here from $@. | SafeUrlFlow.go:10:14:10:21 | selection of Host | here | +| SafeUrlFlow.go:45:24:45:61 | ...+... | SafeUrlFlow.go:37:13:37:19 | selection of URL | SafeUrlFlow.go:45:24:45:61 | ...+... | A safe URL flows here from $@. | SafeUrlFlow.go:37:13:37:19 | selection of URL | here | +| SafeUrlFlow.go:46:29:46:55 | ...+... | SafeUrlFlow.go:37:13:37:19 | selection of URL | SafeUrlFlow.go:46:29:46:55 | ...+... | A safe URL flows here from $@. | SafeUrlFlow.go:37:13:37:19 | selection of URL | here | +| SafeUrlFlow.go:47:11:47:42 | ...+... | SafeUrlFlow.go:37:13:37:19 | selection of URL | SafeUrlFlow.go:47:11:47:42 | ...+... | A safe URL flows here from $@. | SafeUrlFlow.go:37:13:37:19 | selection of URL | here | +| SafeUrlFlow.go:57:11:57:26 | call to String | SafeUrlFlow.go:54:13:54:19 | selection of URL | SafeUrlFlow.go:57:11:57:26 | call to String | A safe URL flows here from $@. | SafeUrlFlow.go:54:13:54:19 | selection of URL | here | +| SafeUrlFlow.go:58:12:58:27 | call to String | SafeUrlFlow.go:54:13:54:19 | selection of URL | SafeUrlFlow.go:58:12:58:27 | call to String | A safe URL flows here from $@. | SafeUrlFlow.go:54:13:54:19 | selection of URL | here | +| SafeUrlFlow.go:59:16:59:31 | call to String | SafeUrlFlow.go:54:13:54:19 | selection of URL | SafeUrlFlow.go:59:16:59:31 | call to String | A safe URL flows here from $@. | SafeUrlFlow.go:54:13:54:19 | selection of URL | here | +| SafeUrlFlow.go:60:12:60:27 | call to String | SafeUrlFlow.go:54:13:54:19 | selection of URL | SafeUrlFlow.go:60:12:60:27 | call to String | A safe URL flows here from $@. | SafeUrlFlow.go:54:13:54:19 | selection of URL | here | +| SafeUrlFlow.go:64:13:64:28 | call to String | SafeUrlFlow.go:54:13:54:19 | selection of URL | SafeUrlFlow.go:64:13:64:28 | call to String | A safe URL flows here from $@. | SafeUrlFlow.go:54:13:54:19 | selection of URL | here | +| SafeUrlFlow.go:65:14:65:29 | call to String | SafeUrlFlow.go:54:13:54:19 | selection of URL | SafeUrlFlow.go:65:14:65:29 | call to String | A safe URL flows here from $@. | SafeUrlFlow.go:54:13:54:19 | selection of URL | here | +| SafeUrlFlow.go:66:18:66:33 | call to String | SafeUrlFlow.go:54:13:54:19 | selection of URL | SafeUrlFlow.go:66:18:66:33 | call to String | A safe URL flows here from $@. | SafeUrlFlow.go:54:13:54:19 | selection of URL | here | +| SafeUrlFlow.go:67:14:67:29 | call to String | SafeUrlFlow.go:54:13:54:19 | selection of URL | SafeUrlFlow.go:67:14:67:29 | call to String | A safe URL flows here from $@. | SafeUrlFlow.go:54:13:54:19 | selection of URL | here | +| SafeUrlFlow.go:70:39:70:54 | call to String | SafeUrlFlow.go:54:13:54:19 | selection of URL | SafeUrlFlow.go:70:39:70:54 | call to String | A safe URL flows here from $@. | SafeUrlFlow.go:54:13:54:19 | selection of URL | here | +| SafeUrlFlow.go:74:70:74:85 | call to String | SafeUrlFlow.go:54:13:54:19 | selection of URL | SafeUrlFlow.go:74:70:74:85 | call to String | A safe URL flows here from $@. | SafeUrlFlow.go:54:13:54:19 | selection of URL | here | +| SafeUrlFlow.go:78:40:78:55 | call to String | SafeUrlFlow.go:54:13:54:19 | selection of URL | SafeUrlFlow.go:78:40:78:55 | call to String | A safe URL flows here from $@. | SafeUrlFlow.go:54:13:54:19 | selection of URL | here | +| SafeUrlFlow.go:89:24:89:41 | call to String | SafeUrlFlow.go:84:14:84:21 | selection of Host | SafeUrlFlow.go:89:24:89:41 | call to String | A safe URL flows here from $@. | SafeUrlFlow.go:84:14:84:21 | selection of Host | here | +| SafeUrlFlow.go:109:11:109:23 | reconstructed | SafeUrlFlow.go:100:13:100:19 | selection of URL | SafeUrlFlow.go:109:11:109:23 | reconstructed | A safe URL flows here from $@. | SafeUrlFlow.go:100:13:100:19 | selection of URL | here | +| SafeUrlFlow.go:112:24:112:50 | ...+... | SafeUrlFlow.go:100:13:100:19 | selection of URL | SafeUrlFlow.go:112:24:112:50 | ...+... | A safe URL flows here from $@. | SafeUrlFlow.go:100:13:100:19 | selection of URL | here | +| SafeUrlFlow.go:113:29:113:58 | ...+... | SafeUrlFlow.go:100:13:100:19 | selection of URL | SafeUrlFlow.go:113:29:113:58 | ...+... | A safe URL flows here from $@. | SafeUrlFlow.go:100:13:100:19 | selection of URL | here | +| SafeUrlFlow.go:114:12:114:42 | ...+... | SafeUrlFlow.go:100:13:100:19 | selection of URL | SafeUrlFlow.go:114:12:114:42 | ...+... | A safe URL flows here from $@. | SafeUrlFlow.go:100:13:100:19 | selection of URL | here | +| SafeUrlFlow.go:115:12:115:25 | safeOpaquePart | SafeUrlFlow.go:100:13:100:19 | selection of URL | SafeUrlFlow.go:115:12:115:25 | safeOpaquePart | A safe URL flows here from $@. | SafeUrlFlow.go:100:13:100:19 | selection of URL | here | +edges +| SafeUrlFlow.go:10:14:10:21 | selection of Host | SafeUrlFlow.go:11:24:11:50 | ...+... | provenance | Sink:MaD:1 | +| SafeUrlFlow.go:10:14:10:21 | selection of Host | SafeUrlFlow.go:17:19:17:26 | safeHost | provenance | | +| SafeUrlFlow.go:13:13:13:19 | selection of URL | SafeUrlFlow.go:14:29:14:35 | safeURL | provenance | Src:MaD:2 | +| SafeUrlFlow.go:14:29:14:35 | safeURL | SafeUrlFlow.go:14:29:14:44 | call to String | provenance | MaD:3 | +| SafeUrlFlow.go:17:19:17:26 | safeHost | SafeUrlFlow.go:18:11:18:19 | targetURL | provenance | Config | +| SafeUrlFlow.go:18:11:18:19 | targetURL | SafeUrlFlow.go:18:11:18:28 | call to String | provenance | MaD:3 | +| SafeUrlFlow.go:37:13:37:19 | selection of URL | SafeUrlFlow.go:45:24:45:61 | ...+... | provenance | Src:MaD:2 Sink:MaD:1 | +| SafeUrlFlow.go:37:13:37:19 | selection of URL | SafeUrlFlow.go:46:29:46:55 | ...+... | provenance | Src:MaD:2 | +| SafeUrlFlow.go:37:13:37:19 | selection of URL | SafeUrlFlow.go:47:11:47:42 | ...+... | provenance | Src:MaD:2 | +| SafeUrlFlow.go:54:13:54:19 | selection of URL | SafeUrlFlow.go:57:11:57:17 | safeURL | provenance | Src:MaD:2 | +| SafeUrlFlow.go:54:13:54:19 | selection of URL | SafeUrlFlow.go:58:12:58:18 | safeURL | provenance | Src:MaD:2 | +| SafeUrlFlow.go:54:13:54:19 | selection of URL | SafeUrlFlow.go:59:16:59:22 | safeURL | provenance | Src:MaD:2 | +| SafeUrlFlow.go:54:13:54:19 | selection of URL | SafeUrlFlow.go:60:12:60:18 | safeURL | provenance | Src:MaD:2 | +| SafeUrlFlow.go:54:13:54:19 | selection of URL | SafeUrlFlow.go:64:13:64:19 | safeURL | provenance | Src:MaD:2 | +| SafeUrlFlow.go:54:13:54:19 | selection of URL | SafeUrlFlow.go:65:14:65:20 | safeURL | provenance | Src:MaD:2 | +| SafeUrlFlow.go:54:13:54:19 | selection of URL | SafeUrlFlow.go:66:18:66:24 | safeURL | provenance | Src:MaD:2 | +| SafeUrlFlow.go:54:13:54:19 | selection of URL | SafeUrlFlow.go:67:14:67:20 | safeURL | provenance | Src:MaD:2 | +| SafeUrlFlow.go:54:13:54:19 | selection of URL | SafeUrlFlow.go:70:39:70:45 | safeURL | provenance | Src:MaD:2 | +| SafeUrlFlow.go:54:13:54:19 | selection of URL | SafeUrlFlow.go:74:70:74:76 | safeURL | provenance | Src:MaD:2 | +| SafeUrlFlow.go:54:13:54:19 | selection of URL | SafeUrlFlow.go:78:40:78:46 | safeURL | provenance | Src:MaD:2 | +| SafeUrlFlow.go:57:11:57:17 | safeURL | SafeUrlFlow.go:57:11:57:26 | call to String | provenance | MaD:3 | +| SafeUrlFlow.go:58:12:58:18 | safeURL | SafeUrlFlow.go:58:12:58:27 | call to String | provenance | MaD:3 | +| SafeUrlFlow.go:59:16:59:22 | safeURL | SafeUrlFlow.go:59:16:59:31 | call to String | provenance | MaD:3 | +| SafeUrlFlow.go:60:12:60:18 | safeURL | SafeUrlFlow.go:60:12:60:27 | call to String | provenance | MaD:3 | +| SafeUrlFlow.go:64:13:64:19 | safeURL | SafeUrlFlow.go:64:13:64:28 | call to String | provenance | MaD:3 | +| SafeUrlFlow.go:65:14:65:20 | safeURL | SafeUrlFlow.go:65:14:65:29 | call to String | provenance | MaD:3 | +| SafeUrlFlow.go:66:18:66:24 | safeURL | SafeUrlFlow.go:66:18:66:33 | call to String | provenance | MaD:3 | +| SafeUrlFlow.go:67:14:67:20 | safeURL | SafeUrlFlow.go:67:14:67:29 | call to String | provenance | MaD:3 | +| SafeUrlFlow.go:70:39:70:45 | safeURL | SafeUrlFlow.go:70:39:70:54 | call to String | provenance | MaD:3 | +| SafeUrlFlow.go:74:70:74:76 | safeURL | SafeUrlFlow.go:74:70:74:85 | call to String | provenance | MaD:3 | +| SafeUrlFlow.go:78:40:78:46 | safeURL | SafeUrlFlow.go:78:40:78:55 | call to String | provenance | MaD:3 | +| SafeUrlFlow.go:84:14:84:21 | selection of Host | SafeUrlFlow.go:87:19:87:26 | safeHost | provenance | | +| SafeUrlFlow.go:87:19:87:26 | safeHost | SafeUrlFlow.go:89:24:89:32 | targetURL | provenance | Config | +| SafeUrlFlow.go:89:24:89:32 | targetURL | SafeUrlFlow.go:89:24:89:41 | call to String | provenance | MaD:3 Sink:MaD:1 | +| SafeUrlFlow.go:100:13:100:19 | selection of URL | SafeUrlFlow.go:109:11:109:23 | reconstructed | provenance | Src:MaD:2 | +| SafeUrlFlow.go:100:13:100:19 | selection of URL | SafeUrlFlow.go:112:24:112:50 | ...+... | provenance | Src:MaD:2 Sink:MaD:1 | +| SafeUrlFlow.go:100:13:100:19 | selection of URL | SafeUrlFlow.go:113:29:113:58 | ...+... | provenance | Src:MaD:2 | +| SafeUrlFlow.go:100:13:100:19 | selection of URL | SafeUrlFlow.go:114:12:114:42 | ...+... | provenance | Src:MaD:2 | +| SafeUrlFlow.go:100:13:100:19 | selection of URL | SafeUrlFlow.go:115:12:115:25 | safeOpaquePart | provenance | Src:MaD:2 | +models +| 1 | Sink: net/http; ; false; Redirect; ; ; Argument[2]; url-redirection[0]; manual | +| 2 | Source: net/http; Request; true; URL; ; ; ; remote; manual | +| 3 | Summary: fmt; Stringer; true; String; ; ; Argument[receiver]; ReturnValue; taint; manual | +nodes +| SafeUrlFlow.go:10:14:10:21 | selection of Host | semmle.label | selection of Host | +| SafeUrlFlow.go:11:24:11:50 | ...+... | semmle.label | ...+... | +| SafeUrlFlow.go:13:13:13:19 | selection of URL | semmle.label | selection of URL | +| SafeUrlFlow.go:14:29:14:35 | safeURL | semmle.label | safeURL | +| SafeUrlFlow.go:14:29:14:44 | call to String | semmle.label | call to String | +| SafeUrlFlow.go:17:19:17:26 | safeHost | semmle.label | safeHost | +| SafeUrlFlow.go:18:11:18:19 | targetURL | semmle.label | targetURL | +| SafeUrlFlow.go:18:11:18:28 | call to String | semmle.label | call to String | +| SafeUrlFlow.go:37:13:37:19 | selection of URL | semmle.label | selection of URL | +| SafeUrlFlow.go:45:24:45:61 | ...+... | semmle.label | ...+... | +| SafeUrlFlow.go:46:29:46:55 | ...+... | semmle.label | ...+... | +| SafeUrlFlow.go:47:11:47:42 | ...+... | semmle.label | ...+... | +| SafeUrlFlow.go:54:13:54:19 | selection of URL | semmle.label | selection of URL | +| SafeUrlFlow.go:57:11:57:17 | safeURL | semmle.label | safeURL | +| SafeUrlFlow.go:57:11:57:26 | call to String | semmle.label | call to String | +| SafeUrlFlow.go:58:12:58:18 | safeURL | semmle.label | safeURL | +| SafeUrlFlow.go:58:12:58:27 | call to String | semmle.label | call to String | +| SafeUrlFlow.go:59:16:59:22 | safeURL | semmle.label | safeURL | +| SafeUrlFlow.go:59:16:59:31 | call to String | semmle.label | call to String | +| SafeUrlFlow.go:60:12:60:18 | safeURL | semmle.label | safeURL | +| SafeUrlFlow.go:60:12:60:27 | call to String | semmle.label | call to String | +| SafeUrlFlow.go:64:13:64:19 | safeURL | semmle.label | safeURL | +| SafeUrlFlow.go:64:13:64:28 | call to String | semmle.label | call to String | +| SafeUrlFlow.go:65:14:65:20 | safeURL | semmle.label | safeURL | +| SafeUrlFlow.go:65:14:65:29 | call to String | semmle.label | call to String | +| SafeUrlFlow.go:66:18:66:24 | safeURL | semmle.label | safeURL | +| SafeUrlFlow.go:66:18:66:33 | call to String | semmle.label | call to String | +| SafeUrlFlow.go:67:14:67:20 | safeURL | semmle.label | safeURL | +| SafeUrlFlow.go:67:14:67:29 | call to String | semmle.label | call to String | +| SafeUrlFlow.go:70:39:70:45 | safeURL | semmle.label | safeURL | +| SafeUrlFlow.go:70:39:70:54 | call to String | semmle.label | call to String | +| SafeUrlFlow.go:74:70:74:76 | safeURL | semmle.label | safeURL | +| SafeUrlFlow.go:74:70:74:85 | call to String | semmle.label | call to String | +| SafeUrlFlow.go:78:40:78:46 | safeURL | semmle.label | safeURL | +| SafeUrlFlow.go:78:40:78:55 | call to String | semmle.label | call to String | +| SafeUrlFlow.go:84:14:84:21 | selection of Host | semmle.label | selection of Host | +| SafeUrlFlow.go:87:19:87:26 | safeHost | semmle.label | safeHost | +| SafeUrlFlow.go:89:24:89:32 | targetURL | semmle.label | targetURL | +| SafeUrlFlow.go:89:24:89:41 | call to String | semmle.label | call to String | +| SafeUrlFlow.go:100:13:100:19 | selection of URL | semmle.label | selection of URL | +| SafeUrlFlow.go:109:11:109:23 | reconstructed | semmle.label | reconstructed | +| SafeUrlFlow.go:112:24:112:50 | ...+... | semmle.label | ...+... | +| SafeUrlFlow.go:113:29:113:58 | ...+... | semmle.label | ...+... | +| SafeUrlFlow.go:114:12:114:42 | ...+... | semmle.label | ...+... | +| SafeUrlFlow.go:115:12:115:25 | safeOpaquePart | semmle.label | safeOpaquePart | +subpaths diff --git a/go/ql/test/library-tests/semmle/go/security/SafeUrlFlow/SafeUrlFlow.go b/go/ql/test/library-tests/semmle/go/security/SafeUrlFlow/SafeUrlFlow.go new file mode 100644 index 000000000000..9a1b2e5677ef --- /dev/null +++ b/go/ql/test/library-tests/semmle/go/security/SafeUrlFlow/SafeUrlFlow.go @@ -0,0 +1,126 @@ +package main + +import ( + "context" + "net/http" + "net/url" +) + +func testStdlibSources(w http.ResponseWriter, req *http.Request) { + safeHost := req.Host // $ Source + http.Redirect(w, req, "https://"+safeHost+"/path", http.StatusFound) // $ Alert + + safeURL := req.URL // $ Source + w.Header().Set("Location", safeURL.String()) // $ Alert + + targetURL := url.URL{} + targetURL.Host = safeHost // URL is safe if Host is safe + http.Get(targetURL.String()) // $ Alert +} + +func testBarrierEdge1(w http.ResponseWriter, req *http.Request) { + safeURL := req.URL + + query := safeURL.Query() // query is not guaranteed to be safe + http.Redirect(w, req, query.Get("redirect"), http.StatusFound) // not guaranteed to be safe +} + +func testBarrierEdge2(w http.ResponseWriter, req *http.Request) { + safeURL := req.URL + + urlString := safeURL.String() + sliced := urlString[0:10] // a substring of a safe URL is not guaranteed to be safe + w.Header().Set("Location", sliced) // not guaranteed to be safe +} + +func testFieldReads(w http.ResponseWriter, req *http.Request) { + safeURL := req.URL // $ Source + + safeScheme := safeURL.Scheme // the scheme of a safe URL is safe + safeHost := safeURL.Host // the host of a safe URL is safe + safePath := safeURL.Path // the path of a safe URL is safe + fragment := safeURL.Fragment // the fragment of a safe URL is not guaranteed to be safe + user := safeURL.User // the user of a safe URL is not guaranteed to be safe + + http.Redirect(w, req, "https://"+safeScheme+"://example.com", http.StatusFound) // $ Alert + w.Header().Set("Location", "https://"+safeHost+"/path") // $ Alert + http.Get("https://example.com" + safePath) // $ Alert + + http.Get(fragment) // not guaranteed to be safe + http.Get(user.String()) // not guaranteed to be safe +} + +func testRequestForgerySinks(req *http.Request) { + safeURL := req.URL // $ Source + + // Standard library HTTP functions (request-forgery sinks) + http.Get(safeURL.String()) // $ Alert + http.Post(safeURL.String(), "application/json", nil) // $ Alert + http.PostForm(safeURL.String(), nil) // $ Alert + http.Head(safeURL.String()) // $ Alert + + // HTTP Client methods (request-forgery sinks) + client := &http.Client{} + client.Get(safeURL.String()) // $ Alert + client.Post(safeURL.String(), "application/json", nil) // $ Alert + client.PostForm(safeURL.String(), nil) // $ Alert + client.Head(safeURL.String()) // $ Alert + + // NewRequest + Client.Do (request-forgery sinks) + request, _ := http.NewRequest("GET", safeURL.String(), nil) // $ Alert + client.Do(request) + + // NewRequestWithContext + Client.Do (request-forgery sinks) + reqWithCtx, _ := http.NewRequestWithContext(context.TODO(), "POST", safeURL.String(), nil) // $ Alert + client.Do(reqWithCtx) + + // RoundTrip method (request-forgery sink) + request2, _ := http.NewRequest("GET", safeURL.String(), nil) // $ Alert + transport := &http.Transport{} + transport.RoundTrip(request2) +} + +func testHostFieldAssignmentFlow(w http.ResponseWriter, req *http.Request) { + safeHost := req.Host // $ Source + + targetURL, _ := url.Parse("http://example.com/data") + targetURL.Host = safeHost // URL is safe if Host is safe + + http.Redirect(w, req, targetURL.String(), http.StatusFound) // $ Alert +} + +func testHostFieldOverwritten(w http.ResponseWriter, req *http.Request) { + safeURL := req.URL + + safeURL.Host = "something.else.com" // safeURL is not guaranteed to be safe now that Host is overwritten + http.Get(safeURL.String()) // not guaranteed to be safe +} + +func testFieldAccess(w http.ResponseWriter, req *http.Request) { + safeURL := req.URL // $ Source + + safeHost := safeURL.Host // the host of a safe URL is safe + safePath := safeURL.Path // the path of a safe URL is safe + safeScheme := safeURL.Scheme // the scheme of a safe URL is safe + safeOpaquePart := safeURL.Opaque // the opaque part of a safe URL is safe + + // Reconstruct URL - still guaranteed to be safe + reconstructed := safeScheme + "://" + safeHost + safePath + http.Get(reconstructed) // $ Alert + + // Test individual fields + http.Redirect(w, req, "https://"+safeHost+"/path", http.StatusFound) // $ Alert + w.Header().Set("Location", "https://example.com"+safePath) // $ Alert + http.Post(safeScheme+"://example.com/api", "application/json", nil) // $ Alert + http.Post(safeOpaquePart, "application/json", nil) // $ Alert + + user := safeURL.User // the user of a safe URL is not guaranteed to be safe + query := safeURL.RawQuery // the query of a safe URL is not guaranteed to be safe + fragment := safeURL.Fragment // the fragment of a safe URL is not guaranteed to be safe + + if user != nil { + http.Redirect(w, req, user.String(), http.StatusFound) // not guaranteed to be safe + } + w.Header().Set("Location", "https://example.com/?"+query) // not guaranteed to be safe + http.Get("https://example.com/#" + fragment) // not guaranteed to be safe +} diff --git a/go/ql/test/library-tests/semmle/go/security/SafeUrlFlow/SafeUrlFlow.ql b/go/ql/test/library-tests/semmle/go/security/SafeUrlFlow/SafeUrlFlow.ql new file mode 100644 index 000000000000..badc69f386cb --- /dev/null +++ b/go/ql/test/library-tests/semmle/go/security/SafeUrlFlow/SafeUrlFlow.ql @@ -0,0 +1,15 @@ +/** + * @id go/test-safe-url-flow + * @kind path-problem + * @problem.severity recommendation + */ + +import go +import semmle.go.security.RequestForgeryCustomizations +import semmle.go.security.OpenUrlRedirectCustomizations +import semmle.go.security.SafeUrlFlow +import SafeUrlFlow::Flow::PathGraph + +from SafeUrlFlow::Flow::PathNode source, SafeUrlFlow::Flow::PathNode sink +where SafeUrlFlow::Flow::flowPath(source, sink) +select sink.getNode(), source, sink, "A safe URL flows here from $@.", source.getNode(), "here" diff --git a/go/ql/test/library-tests/semmle/go/security/SafeUrlFlow/SafeUrlFlow.qlref b/go/ql/test/library-tests/semmle/go/security/SafeUrlFlow/SafeUrlFlow.qlref new file mode 100644 index 000000000000..db1b80a6317c --- /dev/null +++ b/go/ql/test/library-tests/semmle/go/security/SafeUrlFlow/SafeUrlFlow.qlref @@ -0,0 +1,4 @@ +query: SafeUrlFlow.ql +postprocess: + - utils/test/PrettyPrintModels.ql + - utils/test/InlineExpectationsTestQuery.ql