Repository navigation
fix(persist): bound the child processes and stop under-reporting the version - #7
Merged
Merged
Conversation
…version
Four findings from deploying the console locally and driving the whole
route table over raw sockets, none of them exploitable, all of them the
kind of thing that is only visible with the service actually running.
persist: schtasks and the WMI PowerShell query ran under exec.Command,
which waits with no deadline. The Task Scheduler and RPCSS are famous for
wedging, and when they do the handler holds its goroutine until they come
back — a request that never answers instead of an error. A full List()
measures ~5.5s on a stock workstation, so the budget is 20s: a liveness
bound, not a latency target. WaitDelay matters as much as the deadline,
because killing `cmd /c` or powershell does not kill what they spawned and
the inherited pipe stays open until the grandchild exits.
persist: the test has teeth. With the context removed it fails at 9.09s
("the context was not enforced"); with it, 0.45s. `timeout /t` cannot be
the sleep here — it refuses to run without a console, which is exactly how
runHidden spawns its children — so it is a real ping.
server: the validHost comment claimed the stdlib HTTP server rejects the
malformed Host shapes before a handler sees them. Driving the live
listener showed that holds for "127.0.0.1:8080/evil" (400, never reaches
us) but not for "127.0.0.1:8a08" or "127.0.0.1:" — both reach the handler
and are refused by isPort alone. Still not a bypass (a rebinding page
supplies its own hostname, it cannot make the browser send a loopback
one), but the doc was describing a belt that is not there, and the guard
deserves to be trusted for what it does rather than for what the stdlib
does alongside it.
cmd: `--version` from a source build reported 2.0.1 while main is past the
2.0.2 release, so the audit JSON and bug reports carried an older version
than anything published. Bumped, with a note that it tracks the newest
tag. oblpack had no -version at all, which is fine for `go run` and not
fine for a binary that ships in the archives, so it gained one — stamped
like the console, or it would have been a shipped binary inventing its own
version.
DevCop95
added a commit
that referenced
this pull request
Oct 17, 2026
…ling a failed render (#9) I shipped this page claiming "every href is checked against this repo's own asset prefix". The check was a startsWith on the raw string, and that is not the same property. This URL starts with the prefix and still resolves outside the download path: https://github.com/DevCop95/pullgoscript/releases/download/v9/../../../evil -> https://github.com/DevCop95/evil Found by running the shipped file, not by reading it: site.js was exercised in node against a stub DOM with the real API payload, the real checksums.txt, and a hostile payload. Before this commit that hostile payload yielded two rows, one of them pointing at a path the comment claimed was impossible. Not exploitable as shipped, and worth saying plainly: browser_download_url comes from api.github.com over TLS, so forging it needs either the ability to write releases in this repo (in which case you would simply point the link wherever you wanted) or a broken TLS chain. Nothing a reader can be tricked into. It is a claim/code mismatch, which is the same defect class as the validHost comment fixed in #7, and the fix is two lines: resolve the URL and check the origin and the path it actually lands on. Anything that escapes after resolution is dropped, which fails closed — no row, and the caller falls back to the pinned list. Two user-visible strings were also wrong or imprecise: - "Could not read the release list automatically" was printed when the list had been read perfectly well and simply had no usable asset. It now says "Could not build the live table automatically", which is what happened. - The render guard said "the latest release has no assets" when the filter, not the release, was the reason. It now names the URL check. Re-verified after the change, same harness: the real payload still renders 7 rows with the amd64 SHA-256 matching the published checksums.txt, the hostile payload drops from 2 survivors to 1 (javascript: URL, foreign host and the traversal are all gone, and a raw HTML asset name is still written as text), and the 403 and empty-payload paths still show the pinned list. No href assigned in any scenario fails the prefix assertion.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Four fixes that came out of deploying the console locally and driving the whole
route table over raw sockets (29 routes: Host/Origin/token/flag permutations,
WS handshake, a live broadcast frame, the packer, headless audit). Everything
here is a finding that only shows up with the service actually running — the
unit tests were green the whole time.
Relates to #6, which records the two findings from the same pass that are design
decisions rather than bugs, and are deliberately not in this PR.
persist: the child processes had no deadline
schtasks /Queryand the WMI PowerShell query ran underexec.Command, whichwaits with no deadline. The Task Scheduler and RPCSS are famous for wedging, and
when they do, the HTTP handler holds its goroutine until they come back: a
request that never answers instead of an error. Measured on a stock workstation,
a full
List()takes ~5.5s, so the budget is 20s — a liveness bound, not alatency target.
WaitDelaymatters as much as the deadline here: killingcmd /c …orpowershell does not kill what they spawned, and the inherited pipe stays open
until that grandchild exits, so the context alone would still block on the read.
The test has teeth. With the context removed it fails at 9.09s
(
the context was not enforced); with it, 0.45s.timeout /tcannot be thesleep — it refuses to run without a console, which is exactly how
runHiddenspawns its children — so it is a real
ping -n 10.server: the comment described a belt that is not there
validHostclaimed the stdlib HTTP server rejects malformedHostshapesbefore a handler sees them. Driving the live listener showed that is true for
127.0.0.1:8080/evil(400, never reaches us) and false for127.0.0.1:8a08and127.0.0.1:— both reach the handler and are refused byisPortalone:Still not a bypass — a rebinding page supplies its own hostname, it cannot make
the browser send a loopback one — but for those two shapes the strict check is
the only control, not a hardening layer over the stdlib, and the comment now
says so.
cmd: the version was older than anything published
A source build reported
2.0.1whilemainis past the 2.0.2 release, so--audit --jsonand every bug report carried a version older than the publishedone. Bumped, with a note that it tracks the newest tag.
oblpackhad no-versionat all. Fine forgo run, not fine for a binarythat ships in the archives — so it gained one, and it is stamped in
.goreleaser.ymllike the console is. A shipped binary that invents its ownversion is worse than one that admits it does not know it.
Verification
The last line is the one worth repeating:
oblpackandobliteratusbothreport the injected value, so the goreleaser stamp added here is proven rather
than assumed.