Skip to content

fix(persist): bound the child processes and stop under-reporting the version - #7

Merged
DevCop95 merged 1 commit into
mainfrom
fix/liveness-and-version
Oct 17, 2026
Merged

DevCop95 merged 1 commit into
mainfrom
fix/liveness-and-version

Conversation

@DevCop95

Copy link
Copy Markdown
Owner

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 /Query 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 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 a
latency target.

WaitDelay matters as much as the deadline here: killing cmd /c … or
powershell 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 /t cannot be the
sleep — it refuses to run without a console, which is exactly how runHidden
spawns its children — so it is a real ping -n 10.

server: the comment described a belt that is not there

validHost claimed the stdlib HTTP server rejects malformed Host shapes
before 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 for
127.0.0.1:8a08 and 127.0.0.1: — both reach the handler and are refused by
isPort alone:

Host: 127.0.0.1:8a08  -> 403 forbidden: host   <- refused by isPort
Host: 127.0.0.1:      -> 403 forbidden: host   <- refused by isPort
Host: 127.0.0.1:8080/evil -> 400               <- refused by the stdlib

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.1 while main is past the 2.0.2 release, so
--audit --json and every bug report carried a version older than the published
one. Bumped, with a note that it tracks the newest tag.

oblpack had no -version at all. Fine for go run, not fine for a binary
that ships in the archives — so it gained one, and it is stamped in
.goreleaser.yml like the console is. A shipped binary that invents its own
version is worse than one that admits it does not know it.

Verification

gofmt                                        clean
go vet ./... && GOOS=linux go vet ./...      clean
staticcheck ./...                            clean
go test ./... -count=1                       11/11 packages, 0 FAIL
coverage                                     45.3% (floor 43)
gosec -exclude=G103,G104                     files=37 lines=7298 nosec=47 found=0
gosec type-error gate                        no unexpected entry
build (windows/amd64)                        both binaries link
-ldflags -X main.version=v9.9.9-test         stamps BOTH binaries

The last line is the one worth repeating: oblpack and obliteratus both
report the injected value, so the goreleaser stamp added here is proven rather
than assumed.

…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
DevCop95 merged commit 6abaf9c into main Oct 17, 2026
5 checks passed
@DevCop95
DevCop95 deleted the fix/liveness-and-version branch October 17, 2026 17:26
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant