I keep one rule when I read a security fix. Do not trust the summary, trust the code paths. goshs is a small Go HTTP file server, and it had just shipped a patch for GHSA-wvhv-qcqf-f3cx. The patch locked down the write routes, and the notes said reads were protected. That last sentence is exactly the kind of claim worth testing.
The feature and the guard
goshs supports per-directory access control through a .goshs file. Drop one in a folder and you can require a username and password, or block specific filenames. On a normal download the server walks up from the requested file, finds the effective .goshs, and enforces it before serving a byte. That part worked.
The path the fix forgot
goshs also has a bulk download. You hand it one or more files with the bulk query parameter and it streams them back inside a single zip. The patch that fixed the write routes never touched this read route. The bulk handler sanitized each path against directory traversal and then streamed the bytes straight into the archive. It never called the ACL lookup, and it never checked the password. The guard the rest of the server relied on simply was not on this path.
bash
# /protected has a .goshs that requires auth. The normal download is blocked:
curl -s -o /dev/null -w "%{http_code}\n" http://target:8000/protected/secret.txt
# 401 Unauthorized
# The bulk route ignores the ACL and returns the same file inside a zip:
curl -s "http://target:8000/?bulk&file=/protected/secret.txt" -o loot.zip
unzip -p loot.zip
# secret.txt contents, no credentials required
Both halves of the ACL fell. The auth requirement that returned 401 on a normal request was skipped, and the block list that returned 404 was skipped too. I confirmed it on the current release at the time, v2.1.0.
Why this is a finding and not noise
The original advisory said reads were protected. They were, on the routes named in that report. This is the incomplete fix shape I hunt for. A patch gets scoped to the one proof of concept in front of it and a sibling route that reaches the same data is left behind. The dedup test makes the case on its own: one fix does not remediate both, so the bulk route is a distinct issue, not a duplicate of the patched one.
- Vulnerability: unauthenticated read of ACL-protected files through the bulk download route
- Location: the bulkDownload handler in httpserver/updown.go
- Affected: goshs through v2.1.0
- Severity: High, CVSS 7.5
- Status: published as GHSA-rmxw-pq4x-3fvh, fixed, CVE pending
Verifying the fix
I do not stop at submitted. I read the patch. The maintainer moved the ACL check into the bulk route. For each requested file it now resolves the effective .goshs, runs the auth check, and checks the block list, all before any zip header is written. Four regression tests pin the exact cases: no credentials returns 401, valid credentials return 200, a blocked file returns 404, and an inherited parent ACL returns 401. The loop aborts the whole archive on the first protected file, so you cannot smuggle a guarded file next to a public one.
I also chased the one thing that looked like a leftover. The new code skips the check if reading the .goshs config errors, which is fail open. I traced it to ground. That branch only triggers on a malformed existing config, the normal download path behaves the same way, and an attacker cannot make a defender's config malformed. It is not a new bypass. The bulk route is now exactly as strong as the rest of the server.
The lesson
When a patch says reads are protected, do not accept the sentence. List every route that reads. A zip endpoint, an export, a thumbnail, a preview, a share link. Each one is a separate door, and a fix that locks the front door often leaves one of them open. The fastest finding is usually the sibling path the patch never visited.
A patch note is a claim, not a proof. Sita walked through fire because a claim was not enough on its own. Read the second path before you believe the first.
The advisory is public, the fix is in, and the report is credited. If you run goshs, move past v2.1.0.
No comments yet