Refuse unsafe working directories, ask before installing packages #4
No reviewers
Labels
No labels
bug
documentation
duplicate
enhancement
good first issue
help wanted
invalid
question
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
jay/sysperf!4
Loading…
Reference in a new issue
No description provided.
Delete branch "claude/bench-workdir-safety"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Security fixes for
benchfrom 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.XXXXXXXXwork 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). WithGEEKBENCH=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:Sticky directories such as
/tmpstay allowed; there the rename fails.Ask before installing packages (Low)
[Y/n]. Before, any key started the install.read < /dev/ttyfailed silently andapt-get installran 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 useprintf %s, like the ISP and ASN lines.README
Documents both checks: the requirements section, the disk section and troubleshooting.
Testing (as root unless noted)
nobody/tmp)nobodyin their own directoryThe package prompt was tested with
sysbenchhidden fromPATHandapt-getreplaced by a logging stub:apt-getnot calledn: declined,apt-getnot calledyes:apt-get updateandinstallcalled, then the benchmark startsbash -npasses, and shellcheck shows no new warnings.Not in this PR
site/README.mdin jacboy/sysperf#3 and handled separately.Generated by Claude Code
@copilot review these changes
Finding: bench:60–62 treats a failed
readas consent. Pressing Ctrl-D returns an error with an empty answer, which startsapt-getwithout confirmation. Abort whenreadfails; accept an empty answer only when the read succeeds.Confirmed and fixed in
6c5dfad. Pressing Ctrl+D at the[Y/n]prompt madereadfail with an empty answer. That matched the "Enter means yes" case and startedapt-get; I reproduced this by sending Ctrl+D through a pseudo-terminal.The script now stops when
readfails, and an empty answer only counts as yes after a successful read. Results, withsysbenchhidden andapt-getreplaced by a logging stub:apt-getnot callednapt-getnot calledapt-getnot calledyesapt-getcalled, then the benchmark startsGenerated by Claude Code