From 184ce2c6dcdd047d6888fd243b9189c797f58d38 Mon Sep 17 00:00:00 2001 From: Christian Cleberg Date: Wed, 15 Jul 2026 11:41:18 -0500 Subject: fix: reject forged subdomains in the media proxy (#9) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: make memcache concurrency-safe and stamp the version at link time memcache was a crash waiting for traffic. Readers touched tempFS without holding mx, while a per-entry goroutine deleted from it under the lock: a concurrent map read and map write, which the runtime treats as a fatal error that recover cannot catch. The option ships in config.example.json and was the one cache key SETUP.md never documented, so it read like a free win to enable. Put every map and field access behind the mutex, and age the whole map from one janitor instead of a goroutine per cached file, each of which looped forever holding its entry alive. mx is now a plain Mutex: every operation here mutates something, and the old code took an RLock to write. Document the option, and cover it with tests that run the readers, writers and janitor concurrently. Split the disk/origin fetch out of DownloadAndSendMedia while there, so the error path returns instead of falling through to write an empty body after the error page. Release.Version was hardcoded to 1.3.2, so images tagged v1.3.6 reported 1.3.2 from --help and /api/instance, and --help linked to the wrong release. Take it from a main.version string the release workflow links in from the git tag. * fix: reject forged subdomains in the media proxy DownloadAndSendMedia built its upstream URL by concatenation, pasting the subdomain segment of the request path straight into the host position. That segment reaches the handler already percent-decoded, so it can carry "@", "#", "?" and "/" — the characters that end a host. A request for /media/file/x@127.0.0.1:8080%2F/f/x.jpg built a URL whose host parsed as 127.0.0.1:8080, with images-wixmp-x demoted to userinfo, letting any caller aim the instance's fetcher at any address it could reach, including services behind the firewall. Validate the label against ^[a-zA-Z0-9-]+$ and refuse anything else with a 400. Rejecting rather than escaping is what closes this: the label is the host, and url.URL passes a host through verbatim, so building the URL structurally is not sufficient on its own. DeviantArt's own media URLs use a hex-and-dash label, and ParseMedia already splits on the first dot, so a legitimate label cannot contain one. Build the URL from url.URL fields as well, which escapes the path, and encode the token argument, which reached the request unescaped. Reported by CodeQL as go/request-forgery (CWE-918). --- main.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) (limited to 'main.go') diff --git a/main.go b/main.go index 7a5a63e..0d7f1f8 100755 --- a/main.go +++ b/main.go @@ -8,8 +8,17 @@ import ( "github.com/zerolabsco/devianter" ) +// version is the release this binary was built from. The release workflow links +// it in from the git tag so that --help and /api/instance cannot drift from the +// tag the image was built at: +// +// go build -ldflags "-X main.version=1.3.7" +// +// A plain `go build` leaves it as "dev". +var version = "dev" + func main() { - app.Release.Version = "1.3.2" + app.Release.Version = version app.Release.Description = "Two API endpoints and template embedding into binary" app.ExecuteCommandLineArguments() -- cgit v1.2.3