Skip to content

Go: Add tests for SafeUrlFlow, and fix a latent bug #20556

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Oct 1, 2025
Merged

Go: Add tests for SafeUrlFlow, and fix a latent bug #20556

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b5fda88
Remove duplication of `UnsafeFieldReadSanitizer`
owen-mc Sep 30, 2025
5b07e8c
Fix bug in `UnsafeFieldReadSanitizer`
owen-mc Sep 30, 2025
a2a9575
Add tests for safe URL flow
owen-mc Sep 30, 2025
dd3f754
Add change note.
owen-mc Sep 30, 2025
c93852d
Improve comments in test file
owen-mc Oct 1, 2025
8983ac9
Phrase test in terms of safe URLs
owen-mc Oct 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions go/ql/lib/change-notes/2025-09-30-fewer-safe-urls.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
17 changes: 1 addition & 16 deletions go/ql/lib/semmle/go/security/OpenUrlRedirectCustomizations.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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`.
Expand Down
17 changes: 1 addition & 16 deletions go/ql/lib/semmle/go/security/RequestForgeryCustomizations.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
)
}
}
6 changes: 4 additions & 2 deletions go/ql/lib/semmle/go/security/SafeUrlFlow.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 15 additions & 0 deletions go/ql/lib/semmle/go/security/SafeUrlFlowCustomizations.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
)
}
}
}
115 changes: 115 additions & 0 deletions go/ql/test/library-tests/semmle/go/security/SafeUrlFlow/SafeUrlFlow.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Loading