Skip to content

Thanks + Audit Results #39

Description

@MicahZoltu

I would like to to say thanks for writing good software, it is rare these days. I cloned your repository and asked my agent to audit it for privacy and security issues. The only things of significance it found were related to the server bit, which is disabled by default. Unlike most software I audit, this one was nice and clean, with no third party dependencies bloating things, no analytics, and generally just good lightweight code.

I was even able to get it building inside a Linux docker container and it ran perfectly on Windows afterwards, which is a testament to how simple the build is.

Feel free to close this issue, I just wanted to say thanks for standing out in a sea of terrible over-engineered and bloated of software.

I'm including the audit results below in case anyone else wants to see them:


Audit scope & integrity

  • Reviewed every tracked file: Main.c, Main.h, Server.c, Server.h, resource.h, UniversalPauseButton.rc, .vcxproj, .sln, .filters, both HTML pages, images, and git metadata.
  • The working tree is clean and HEAD is 469276b4c58a1193db0c793426ac148244f1dbd2, which exactly matches upstream refs/heads/master on github.com/ryanries/UniversalPauseButton (verified via git ls-remote). The code has not been tampered with relative to the published repository.
  • No build scripts, package manifests, or CI workflows exist that fetch/execute third-party code. .git/hooks contains only .sample files. The .vcxproj has no pre/post-build/Exec commands.

Backdoors / exfiltration / phoning home

None found. Specifically:

  • No outbound network calls. The only socket code (Server.c) is a listener; there is no connect(), HTTP client, WinHttp/WinInet/URLDownloadToFile, DNS, or curl usage.
  • No telemetry, auto-update, or external URL in the HTML/JS/platform code. Both main_page.htm and welcome.htm are fully self-contained (grep for http(s)://, fetch, WebSocket, sendBeacon, eval, atob, new Image, external src found only the locally-constructed http://${ip}:${port} and a commented-out window.open).
  • The only URL ever opened is http://localhost:<port>/welcome.htm via ShellExecuteA (Server.c:521-524).
  • AsciiToUnicodeLong/DbgPrint write only to the local debug console. No hidden key combos; the hotkey is user-configurable and range-checked.
  • Images (pause.ico, PNGs) contain no appended payload; the ICO's six embedded images' offsets/sizes account for the whole file.

Actual security vulnerabilities

# Severity Finding Location
1 High (when enabled) Unauthenticated remote-control HTTP service bound to all interfaces (AI_PASSIVE, host NULL, AF_INET). Anyone on the LAN can toggle pause via POST /pause. No auth, token, or rate limiting. Server.c:293-353, 432-483
2 High (when enabled) CSRF / DNS-rebinding: request routing uses strstr(buffer, "POST /pause") with no Host/Origin/Referer/method validation. Any malicious web page can make the user's browser hit http://127.0.0.1:<port>/pause and pause their foreground app/game. Server.c:311, 319, 332
3 Medium Information disclosure: GET /connection-info returns every non-virtual adapter's IPv4/IPv6 addresses (122 bytes+) to any unauthenticated requester. Useful for network reconnaissance; reachable cross-origin via DNS rebinding. Server.c:319-331, 154-280
4 Low Unsafe HTML injection: listItem.innerHTML = \${url}`builds DOM from server data without escaping. Data originates frominet_ntop` so not currently exploitable, but it is a fragile pattern. welcome.htm:347-353
5 Low Potential NULL deref / DoS: wcsstr(pCurrAddresses->Description, ...) and FriendlyName are not NULL-checked; the Win32 docs allow Description to be NULL. Server.c:216-221, 226
6 Low Buffer-accounting bug: write_to_buf advances *buf/*rem_chars by vsnprintf's untruncated return value, not the amount written. On overflow the pointer walks past the destination and rem_chars goes negative. Subsequent writes are skipped by the *rem_chars <= 0 guard, so no OOB write today, but it is incorrect and latent. Server.c:115-148
7 Low Adapter-list logic bug: pCurrAddresses = pCurrAddresses->Next is inside the for (i<2) loop, advancing the list twice per outer iteration (skips adapters / duplicates processing). Correctness, not memory safety. Server.c:230-274
8 Low Signature/arg mismatch & uninitialized handle: openWelcomePageInBrowser() is defined with no params but called with gConfig.WebPort (relies on global). serve_stop waits on/CloseHandles serverThreadHandle, a zero-initialized global that was never created. Harmless today, UB-ish. Main.c:154, Server.c:517, 488-505
9 Info Dangerous primitives / least privilege: suspends arbitrary processes via undocumented ntdll!NtSuspendProcess/NtResumeProcess with PROCESS_ALL_ACCESS. Can pause security software and cause race conditions (acknowledged in README); should run unelevated only when necessary. Main.c:56-70, 231, 266, 295

Notes on lower-value automated hits: the format-string-vuln warnings on Server.c:124/136/521 are false positives (format strings are literals/caller-fixed); memcpy-overlap (Server.c:392) cannot overlap (separate malloc); "insecure HTTP" is inherent to a LAN control service.

Dependency audit

No third-party dependencies, vendored code, or package managers. Links are Windows system libraries only: ws2_32, iphlpapi, ntdll, shell32, user32, kernel32, advapi32 (pragma comment lines Server.c:16-17). Dependency risk therefore reduces to patched Windows OS components and the undocumented ntdll functions (not guaranteed API-stable). The repo ships no compiled binaries, so any release artifact must be treated as a separate supply-chain item not covered by this source audit.

Recommendations

  1. Default (WebPort = 0) disables the server — keep it off unless needed; bind to loopback by default or require an explicit interface choice.
  2. Add authentication (per-session random token surfaced in the welcome URL) and reject requests whose Host/Origin isn't the expected local origin; enforce the HTTP method.
  3. Remove or restrict /connection-info to loopback, and return only the address the client actually connected on.
  4. Escape any value inserted into innerHTML (or use textContent).
  5. Make write_to_buf clamp by the actual written length; NULL-check adapter Description/FriendlyName; fix the double ->Next advance; drop the unused serverThreadHandle wait.
  6. Run unelevated; drop PROCESS_ALL_ACCESS to PROCESS_SUSPEND_RESUME.

Bottom line: This is benign, legitimate software (a game-pause tray app) with no backdoor, exfiltration, or phone-home behavior. Its main real risk is the opt-in web control port, which exposes unauthenticated pause control and local-IP disclosure to the network and is vulnerable to CSRF/DNS-rebinding from any web page.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions