Refuse unsafe working directories, ask before installing packages #4

Merged
jacboy merged 2 commits from claude/bench-workdir-safety into master 2026-11-09 14:19:55 +00:00
jacboy commented 2026-11-09 14:14:02 +00:00 (Migrated from github.com)

Security fixes for bench from a re-audit of the merged script. They're independent of the landing page in jacboy/sysperf#3.

Refuse unsafe working directories (Medium)

The private .sysperf.XXXXXXXX work directory (mode 0700) is created in the current directory. It's only safe if no other user can rename entries on the path to it. In a directory that others can write to without the sticky bit, or one owned by another user, an attacker can rename root's work directory, put their own in its place and add symlinks. That redirects root's 2 GB fio write to any file (e.g. /etc/shadow). With GEEKBENCH=7, it also lets them replace the verified Geekbench binary before it runs.

This was reproduced before the fix: as nobody, swapping root's work directory in a 0777 directory made a root write land in a root-only file.

Fix: before creating the work directory, the script checks every directory from / down to the current one, using the real path (pwd -P). It refuses with a clear message ("run it from a directory only you or root can change, e.g. cd ~") if any of them:

  • is owned by someone other than root or the current user, or
  • is group- or world-writable without the sticky bit.

Sticky directories such as /tmp stay allowed; there the rename fails.

Ask before installing packages (Low)

  • With a terminal: the prompt is now [Y/n]. Before, any key started the install.
  • Without a terminal (cron, CI): before, read < /dev/tty failed silently and apt-get install ran anyway. Now the script prints the install command and exits.

Print system values with printf %s (Informational)

CPU model, OS, kernel and the other system values were printed with echo -e, which interprets backslash sequences in them. They now use printf %s, like the ISP and ASN lines.

README

Documents both checks: the requirements section, the disk section and troubleshooting.

Testing (as root unless noted)

Current directory Result
0777 refused, no work directory created
0775 refused
owned by nobody refused
0755 below a 0777 parent refused, naming the parent
symlink to a 0777 directory refused (real path checked)
1777 (sticky, like /tmp) runs
normal 0755 directory runs
nobody in their own directory runs, and the work directory is cleaned up

The package prompt was tested with sysbench hidden from PATH and apt-get replaced by a logging stub:

  • no terminal: install command printed, apt-get not called
  • answer n: declined, apt-get not called
  • Enter / yes: apt-get update and install called, then the benchmark starts

bash -n passes, and shellcheck shows no new warnings.

Not in this PR

  • Geekbench still runs as root (Low). Dropping privileges needs a full Geekbench run to confirm it still reports scores.
  • The domain and delivery-chain items (DNSSEC, SPF/DMARC, old A/AAAA records, 2FA, branch protection) are documented in site/README.md in jacboy/sysperf#3 and handled separately.

Generated by Claude Code

Security fixes for `bench` from a re-audit of the merged script. They're independent of the landing page in jacboy/sysperf#3. ## Refuse unsafe working directories (Medium) The private `.sysperf.XXXXXXXX` work directory (mode 0700) is created in the current directory. It's only safe if no other user can rename entries on the path to it. In a directory that others can write to without the sticky bit, or one owned by another user, an attacker can rename root's work directory, put their own in its place and add symlinks. That redirects root's 2 GB fio write to any file (e.g. `/etc/shadow`). With `GEEKBENCH=7`, it also lets them replace the verified Geekbench binary before it runs. This was reproduced before the fix: as `nobody`, swapping root's work directory in a 0777 directory made a root write land in a root-only file. **Fix:** before creating the work directory, the script checks every directory from `/` down to the current one, using the real path (`pwd -P`). It refuses with a clear message ("run it from a directory only you or root can change, e.g. `cd ~`") if any of them: - is owned by someone other than root or the current user, or - is group- or world-writable without the sticky bit. Sticky directories such as `/tmp` stay allowed; there the rename fails. ## Ask before installing packages (Low) - **With a terminal:** the prompt is now `[Y/n]`. Before, any key started the install. - **Without a terminal (cron, CI):** before, `read < /dev/tty` failed silently and `apt-get install` ran anyway. Now the script prints the install command and exits. ## Print system values with `printf %s` (Informational) CPU model, OS, kernel and the other system values were printed with `echo -e`, which interprets backslash sequences in them. They now use `printf %s`, like the ISP and ASN lines. ## README Documents both checks: the requirements section, the disk section and troubleshooting. ## Testing (as root unless noted) | Current directory | Result | |---|---| | 0777 | refused, no work directory created | | 0775 | refused | | owned by `nobody` | refused | | 0755 below a 0777 parent | refused, naming the parent | | symlink to a 0777 directory | refused (real path checked) | | 1777 (sticky, like `/tmp`) | runs | | normal 0755 directory | runs | | `nobody` in their own directory | runs, and the work directory is cleaned up | The package prompt was tested with `sysbench` hidden from `PATH` and `apt-get` replaced by a logging stub: - no terminal: install command printed, `apt-get` not called - answer `n`: declined, `apt-get` not called - Enter / `yes`: `apt-get update` and `install` called, then the benchmark starts `bash -n` passes, and shellcheck shows no new warnings. ## Not in this PR - **Geekbench still runs as root** (Low). Dropping privileges needs a full Geekbench run to confirm it still reports scores. - **The domain and delivery-chain items** (DNSSEC, SPF/DMARC, old A/AAAA records, 2FA, branch protection) are documented in `site/README.md` in jacboy/sysperf#3 and handled separately. --- _Generated by [Claude Code](https://claude.ai/code/session_01WFUVi2gu5nYSuZeyRWAGiM)_
jacboy commented 2026-11-09 14:14:45 +00:00 (Migrated from github.com)

@copilot review these changes

@copilot review these changes
Copilot commented 2026-11-09 14:16:37 +00:00 (Migrated from github.com)

@copilot review these changes

Finding: bench:60–62 treats a failed read as consent. Pressing Ctrl-D returns an error with an empty answer, which starts apt-get without confirmation. Abort when read fails; accept an empty answer only when the read succeeds.

> @copilot review these changes **Finding:** [bench:60–62](https://github.com/jacboy/sysperf/blob/f949753b7a4444ef7797e3883439fbace0cac762/bench#L60-L62) treats a failed `read` as consent. Pressing Ctrl-D returns an error with an empty answer, which starts `apt-get` without confirmation. Abort when `read` fails; accept an empty answer only when the read succeeds.
jacboy commented 2026-11-09 14:18:11 +00:00 (Migrated from github.com)

Confirmed and fixed in 6c5dfad. Pressing Ctrl+D at the [Y/n] prompt made read fail with an empty answer. That matched the "Enter means yes" case and started apt-get; I reproduced this by sending Ctrl+D through a pseudo-terminal.

The script now stops when read fails, and an empty answer only counts as yes after a successful read. Results, with sysbench hidden and apt-get replaced by a logging stub:

Input Result
Ctrl+D "No answer, not installing anything", apt-get not called
n declined, apt-get not called
no terminal install command printed, apt-get not called
Enter / yes apt-get called, then the benchmark starts

Generated by Claude Code

Confirmed and fixed in 6c5dfad. Pressing Ctrl+D at the `[Y/n]` prompt made `read` fail with an empty answer. That matched the "Enter means yes" case and started `apt-get`; I reproduced this by sending Ctrl+D through a pseudo-terminal. The script now stops when `read` fails, and an empty answer only counts as yes after a successful read. Results, with `sysbench` hidden and `apt-get` replaced by a logging stub: | Input | Result | |---|---| | Ctrl+D | "No answer, not installing anything", `apt-get` not called | | `n` | declined, `apt-get` not called | | no terminal | install command printed, `apt-get` not called | | Enter / `yes` | `apt-get` called, then the benchmark starts | --- _Generated by [Claude Code](https://claude.ai/code)_
Sign in to join this conversation.
No description provided.