Skip to content

Commit 9c1ec52

Browse files
dhhryanrhughes
authored andcommitted
Merge pull request #8198 from bastidotnet/harden-apple-brightness-device-cache
Validate the cached Apple-display device path before use (cherry picked from commit 06e32d2)
1 parent 880605f commit 9c1ec52

2 files changed

Lines changed: 169 additions & 4 deletions

File tree

bin/omarchy-brightness-display-apple

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,13 @@
44
# omarchy:args=[--no-osd] [+N%|N%-|N%]
55
# omarchy:examples=omarchy brightness display apple | omarchy brightness display apple +5% | omarchy brightness display apple --no-osd 50%
66

7-
device_cache="${XDG_RUNTIME_DIR:-/tmp}/omarchy-brightness-display-apple.device"
7+
# Only cache under the user-private runtime dir. With no XDG_RUNTIME_DIR we skip
8+
# caching (detect every run) rather than fall back to a predictable, world-writable
9+
# /tmp path another user could pre-create.
10+
device_cache=""
11+
if [[ -n ${XDG_RUNTIME_DIR:-} ]]; then
12+
device_cache="$XDG_RUNTIME_DIR/omarchy-brightness-display-apple.device"
13+
fi
814
no_osd=0
915
if [[ ${1:-} == "--no-osd" ]]; then
1016
no_osd=1
@@ -28,9 +34,14 @@ find_apple_display_device() {
2834
local cached=""
2935
local device=""
3036

31-
if [[ -r $device_cache ]]; then
37+
if [[ -n $device_cache && -r $device_cache ]]; then
3238
read -r cached <"$device_cache" || true
33-
if [[ -n $cached && -e $cached ]]; then
39+
# Trust a cached value only if it still names a hiddev character device. A
40+
# stale or unexpected cache (a regular file, a non-hiddev node) is ignored and
41+
# we re-detect instead of handing an arbitrary path to asdcontrol. The globs
42+
# are left unquoted on purpose: [[ ]] pattern-matches an unquoted right side,
43+
# and quoting them would turn the match into a literal string comparison.
44+
if [[ ( $cached == /dev/hiddev* || $cached == /dev/usb/hiddev* ) && -c $cached ]]; then
3445
printf '%s\n' "$cached"
3546
return 0
3647
fi
@@ -39,7 +50,9 @@ find_apple_display_device() {
3950
device="$(detect_apple_display_device)" || return 1
4051
[[ -n $device ]] || return 1
4152

42-
printf '%s\n' "$device" >"$device_cache"
53+
if [[ -n $device_cache ]]; then
54+
printf '%s\n' "$device" >"$device_cache"
55+
fi
4356
printf '%s\n' "$device"
4457
}
4558

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
#!/bin/bash
2+
3+
set -euo pipefail
4+
5+
source "$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)/base-test.sh"
6+
7+
TMPDIR=$(mktemp -d)
8+
# The /tmp-fallback case (below) must place its decoy at exactly the fixed path the
9+
# old wrapper would have formed, so it cannot use a random mktemp name. Track whether
10+
# we created it and remove it on exit only then -- never touch a path we did not create.
11+
tmp_cache="/tmp/omarchy-brightness-display-apple.device"
12+
created_tmp_cache=0
13+
14+
cleanup() {
15+
rm -rf "$TMPDIR"
16+
# Remove the /tmp decoy only if this test is the one that created it.
17+
if (( created_tmp_cache )); then
18+
rm -f "$tmp_cache"
19+
fi
20+
}
21+
trap cleanup EXIT
22+
23+
# Stubs on PATH: drop sudo so asdcontrol runs directly, record every asdcontrol
24+
# invocation, make detection deterministic by having --detect report no device,
25+
# and no-op the OSD. On a host without any /dev/*hiddev* node the wrapper's
26+
# detect_apple_display_device returns before it ever runs asdcontrol, so the
27+
# reject cases assert on the negative: a refused cache value is never handed to
28+
# `asdcontrol <dev> -- <step>`. Blind-trust validation would hand it over and be
29+
# caught here.
30+
stub_dir="$TMPDIR/stubs"
31+
mkdir -p "$stub_dir"
32+
33+
asd_log="$TMPDIR/asdcontrol.log"
34+
35+
cat >"$stub_dir/sudo" <<'STUB'
36+
#!/bin/bash
37+
exec "$@"
38+
STUB
39+
chmod +x "$stub_dir/sudo"
40+
41+
cat >"$stub_dir/asdcontrol" <<STUB
42+
#!/bin/bash
43+
printf '%s\n' "\$*" >>"$asd_log"
44+
# --detect reports nothing, so detection never yields a device.
45+
if [[ \$1 == "--detect" ]]; then
46+
exit 0
47+
fi
48+
# A brightness read (a lone device arg) returns a plausible value; a set
49+
# (<device> -- <step>) just succeeds.
50+
if [[ \$# -eq 1 ]]; then
51+
printf '%s: BRIGHTNESS=30000\n' "\$1"
52+
fi
53+
exit 0
54+
STUB
55+
chmod +x "$stub_dir/asdcontrol"
56+
57+
cat >"$stub_dir/omarchy-osd" <<'STUB'
58+
#!/bin/bash
59+
exit 0
60+
STUB
61+
chmod +x "$stub_dir/omarchy-osd"
62+
63+
run_wrapper() {
64+
# $1: value for XDG_RUNTIME_DIR ("" means unset); remaining args go to the wrapper.
65+
local xdg="$1"
66+
shift
67+
: >"$asd_log"
68+
if [[ -n $xdg ]]; then
69+
XDG_RUNTIME_DIR="$xdg" PATH="$stub_dir:$ROOT/bin:$PATH" \
70+
omarchy-brightness-display-apple "$@" 2>&1 || true
71+
else
72+
env -u XDG_RUNTIME_DIR PATH="$stub_dir:$ROOT/bin:$PATH" \
73+
omarchy-brightness-display-apple "$@" 2>&1 || true
74+
fi
75+
}
76+
77+
# --- A cache value that is not a hiddev character device is rejected ----------
78+
xdg_dir="$TMPDIR/xdg"
79+
mkdir -p "$xdg_dir"
80+
cache_file="$xdg_dir/omarchy-brightness-display-apple.device"
81+
82+
regular_file="$TMPDIR/not-a-device"
83+
: >"$regular_file"
84+
85+
poisons=("/dev/null" "$regular_file" "/tmp/omarchy-evil")
86+
87+
# The cases above all fail on the pathname prefix, so none of them reaches the -c
88+
# test -- drop `&& -c $cached` from the wrapper and they all still pass. A path
89+
# that matches the hiddev glob but is not a character device is what -c is for,
90+
# and it is the realistic stale cache: the display replugs, the interface
91+
# renumbers, and the cached node is simply gone. Add it only when the host really
92+
# has no such node, so a machine with the display attached cannot fail here.
93+
if [[ ! -e /dev/hiddev999 ]]; then
94+
poisons+=("/dev/hiddev999")
95+
fi
96+
97+
for poison in "${poisons[@]}"; do
98+
printf '%s\n' "$poison" >"$cache_file"
99+
output=$(run_wrapper "$xdg_dir" "+5%")
100+
if grep -qF -- "$poison -- +5%" "$asd_log"; then
101+
fail "wrapper handed a non-hiddev cache value to asdcontrol: $poison" "$output"
102+
fi
103+
done
104+
pass "wrapper rejects a cached path that is not a hiddev character device"
105+
106+
# NOTE: the /dev/hiddev999 case above covers the -c test for a glob-matching path
107+
# that does not exist. The remaining arm -- a path under /dev that exists, matches
108+
# the glob, and is not a character device -- cannot be built without root, since
109+
# only real device nodes live there.
110+
111+
# --- A legitimate cached hiddev node is trusted (only where HW is present) ----
112+
real_hiddev=""
113+
for candidate in /dev/usb/hiddev* /dev/hiddev*; do
114+
if [[ -c $candidate ]]; then
115+
real_hiddev="$candidate"
116+
break
117+
fi
118+
done
119+
if [[ -n $real_hiddev ]]; then
120+
printf '%s\n' "$real_hiddev" >"$cache_file"
121+
run_wrapper "$xdg_dir" "+5%" >/dev/null
122+
grep -qF -- "$real_hiddev -- +5%" "$asd_log" ||
123+
fail "wrapper did not trust a valid cached hiddev node: $real_hiddev"
124+
pass "wrapper trusts a cached hiddev character device without re-detecting"
125+
else
126+
pass "no /dev/hiddev* character device present; skipping the valid-cache case"
127+
fi
128+
129+
# --- With no XDG_RUNTIME_DIR, the predictable /tmp cache is not consulted ------
130+
# Assert on the open, not on the contents. A decoy holding a rejectable path proves
131+
# nothing: the validation above refuses it whether or not the /tmp fallback is still
132+
# there, so that assertion passes against both wrappers. A FIFO with no writer blocks
133+
# whoever opens it, so a wrapper that consults the path hangs and one that ignores it
134+
# exits -- which separates the two. mkfifo is atomic and fails outright if the path is
135+
# taken, so it neither overwrites a file nor follows a symlink; the fixed path is
136+
# required, being exactly the path the old code would have formed. Clear the flag as
137+
# soon as the decoy is gone, so a concurrent run's decoy cannot be removed by this
138+
# run's EXIT trap.
139+
if mkfifo "$tmp_cache" 2>/dev/null; then
140+
created_tmp_cache=1
141+
status=0
142+
env -u XDG_RUNTIME_DIR PATH="$stub_dir:$ROOT/bin:$PATH" \
143+
timeout 5 omarchy-brightness-display-apple "+5%" >/dev/null 2>&1 || status=$?
144+
rm -f "$tmp_cache"
145+
created_tmp_cache=0
146+
(( status != 124 )) ||
147+
fail "wrapper consulted the world-writable /tmp cache with no XDG_RUNTIME_DIR" \
148+
"it blocked reading the FIFO decoy at $tmp_cache"
149+
pass "wrapper ignores the /tmp cache path when XDG_RUNTIME_DIR is unset"
150+
else
151+
pass "$tmp_cache already present or not safely creatable; skipping the /tmp-fallback case"
152+
fi

0 commit comments

Comments
 (0)