From 0ad64a59df1c129cc08d183d5ccd857e3c7f36e9 Mon Sep 17 00:00:00 2001 From: KazeTachinuu <90521430+KazeTachinuu@users.noreply.github.com> Date: Tue, 11 Aug 2026 12:31:52 +0200 Subject: [PATCH] Fix command injection in theme install, drop tzupdate NOPASSWD (#6694) * Fix theme install code execution and drop tzupdate NOPASSWD VULN-01 (C, D, E): a malicious theme can execute arbitrary code during install through three injection sinks: C: colors.toml values reach a sed script unsanitized. GNU sed's `e` flag runs the pattern space as a shell command. D: vscode.json `.name` is interpolated into a sed replacement string without escaping sed metacharacters. E: keyboard.rgb content is interpolated into a python3 -c argument without validation. Fix C by validating keys and values in omarchy-theme-color's parser with a character allowlist. Byte-identical output for all 22 shipped themes. Fix D by escaping backslash, ampersand, and slash in the theme name before sed interpolation. Fix E by gating on ^[0-9A-Fa-f]{6}$ before interpolation, in both the Framework 16 and ASUS ROG keyboard scripts. VULN-02: the tzupdate sudoers grant has no argument constraint. tzupdate -l lets any wheel user write a root-owned symlink to any path. Drop it; nothing has invoked tzupdate since omarchy-cmd-tzupdate was removed. Keep timedatectl set-timezone. * Harden keyboard and vscode theme scripts keyboard-f16: pass hex as sys.argv instead of interpolating into python3 -c. The hex validation gate stays as the primary defense; argv separation is defense-in-depth per OWASP guidance. vscode: replace sed interpolation of theme name with jq, which handles arbitrary strings safely via --arg. Validate extension IDs against ^[a-zA-Z0-9._-]+$ before passing to --install-extension. * Keep VS Code settings edits JSONC-safe settings.json is JSONC, so routing the write through jq dropped theme sync entirely for anyone with a comment or trailing comma in the file, including the `{ "workbench.colorTheme": "",\n}` shape Omarchy itself creates. Edit in place again and close the injection by validating the theme label instead. Scope the extension-id guard to the install so a malformed id no longer skips the colorTheme write, and treat a missing descriptor field as empty rather than the literal string "null". Co-Authored-By: Claude Opus 5 * Widen the accepted colors.toml value charset The sanitizer dropped gradient angles, decimals, underscored palette references, and paths, which vanish from --raw/--all and leave a raw {{ placeholder }} in the generated config. Allow the punctuation real palettes use, keep out everything sed treats as special, and say so on stderr rather than dropping a key silently. Co-Authored-By: Claude Opus 5 --------- Co-authored-by: David Heinemeier Hansson Co-authored-by: Claude Opus 5 --- bin/omarchy-theme-color | 15 +++++++++++++++ bin/omarchy-theme-set-keyboard-asus-rog | 6 ++++-- bin/omarchy-theme-set-keyboard-f16 | 9 ++++++--- bin/omarchy-theme-set-vscode | 24 +++++++++++++++++++----- etc/sudoers.d/omarchy-tzupdate | 2 +- test/shell.d/timezone-test.sh | 5 ++++- 6 files changed, 49 insertions(+), 12 deletions(-) diff --git a/bin/omarchy-theme-color b/bin/omarchy-theme-color index cdf2592d..c5d09ca2 100755 --- a/bin/omarchy-theme-color +++ b/bin/omarchy-theme-color @@ -151,6 +151,21 @@ parse_colors_file() { value="${value%"${value##*[![:space:]]}"}" # trim unquoted values fi + # Values reach consumers as sed replacement text, so the charset excludes + # the delimiter, backslash, and & while still covering everything a real + # palette holds: hex, rgb()/rgba() lists, gradient angles like -45deg, + # decimals, and bare words. Rejections are announced so a third-party theme + # doesn't lose a key silently and leave a raw {{ placeholder }} behind. + if [[ ! $key =~ ^[abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_-]+$ ]]; then + printf 'omarchy-theme-color: skipping key with unsupported characters\n' >&2 + continue + fi + + if [[ ! $value =~ ^[abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789#(),._+/%\ -]*$ ]]; then + printf 'omarchy-theme-color: skipping %s: unsupported characters in value\n' "$key" >&2 + continue + fi + THEME_COLORS[$key]="$value" [[ $OUTPUT == "raw" ]] && printf '%s\t%s\n' "$key" "$value" done <"$COLORS_FILE" diff --git a/bin/omarchy-theme-set-keyboard-asus-rog b/bin/omarchy-theme-set-keyboard-asus-rog index 37956214..5dd42ee5 100755 --- a/bin/omarchy-theme-set-keyboard-asus-rog +++ b/bin/omarchy-theme-set-keyboard-asus-rog @@ -5,6 +5,8 @@ ASUSCTL_THEME=$HOME/.local/state/omarchy/current/theme/keyboard.rgb -if omarchy-cmd-present asusctl; then - asusctl aura effect static -c $(sed 's/^#//' $ASUSCTL_THEME) +if omarchy-cmd-present asusctl && [[ -f $ASUSCTL_THEME ]]; then + color=$(sed 's/^#//' "$ASUSCTL_THEME") + [[ $color =~ ^[0-9A-Fa-f]{6}$ ]] || exit 0 + asusctl aura effect static -c "$color" fi diff --git a/bin/omarchy-theme-set-keyboard-f16 b/bin/omarchy-theme-set-keyboard-f16 index fecceed1..0fab5174 100755 --- a/bin/omarchy-theme-set-keyboard-f16 +++ b/bin/omarchy-theme-set-keyboard-f16 @@ -9,13 +9,16 @@ if omarchy-cmd-present qmk_hid && [[ -f $FRAMEWORK16_THEME ]]; then hex=$(cat "$FRAMEWORK16_THEME") hex="${hex#\#}" + [[ $hex =~ ^[0-9A-Fa-f]{6}$ ]] || exit 0 + # Convert hex to QMK HSV (0-255 scale) using Python's colorsys read -r h s <<< $(python3 -c " -import colorsys -r, g, b = int('$hex'[:2],16)/255, int('$hex'[2:4],16)/255, int('$hex'[4:6],16)/255 +import sys, colorsys +hex = sys.argv[1] +r, g, b = int(hex[:2],16)/255, int(hex[2:4],16)/255, int(hex[4:6],16)/255 h, s, v = colorsys.rgb_to_hsv(r, g, b) print(int(h * 255), int(s * 255)) -") +" "$hex") qmk_hid via --rgb-effect 1 2>/dev/null qmk_hid via --rgb-hue "$h" 2>/dev/null diff --git a/bin/omarchy-theme-set-vscode b/bin/omarchy-theme-set-vscode index c7daef73..d91b8ba0 100755 --- a/bin/omarchy-theme-set-vscode +++ b/bin/omarchy-theme-set-vscode @@ -103,12 +103,20 @@ set_theme() { local theme_name="" if [[ -f "$VS_CODE_THEME_DESCRIPTOR" ]]; then - # Theme specifies a preferred 3rd-party extension/theme pair. - theme_name=$(jq -r '.name' "$VS_CODE_THEME_DESCRIPTOR") + # Theme specifies a preferred 3rd-party extension/theme pair. Both fields + # come from the theme, so neither is trusted as a shell or sed argument. local extension - extension=$(jq -r '.extension' "$VS_CODE_THEME_DESCRIPTOR") + theme_name=$(jq -r '.name // empty' "$VS_CODE_THEME_DESCRIPTOR") + extension=$(jq -r '.extension // empty' "$VS_CODE_THEME_DESCRIPTOR") - if [[ -n $extension ]] && ! "$editor_cmd" --list-extensions 2>/dev/null | grep -Fxq "$extension"; then + # A label with quotes, backslashes, or control characters can't land in + # settings.json as valid JSON, so drop it rather than write it out. + [[ $theme_name =~ ^[^[:cntrl:]\"\\]+$ ]] || theme_name="" + + # Only install a well-formed publisher.name id; a malformed one skips the + # install without skipping the colorTheme write below. + if [[ $extension =~ ^[a-zA-Z0-9._-]+$ ]] && + ! "$editor_cmd" --list-extensions 2>/dev/null | grep -Fxq "$extension"; then "$editor_cmd" --install-extension "$extension" >/dev/null 2>&1 fi elif [[ -f "$GENERATED_THEME" ]]; then @@ -121,12 +129,18 @@ set_theme() { mkdir -p "$(dirname "$settings_path")" [[ -f $settings_path ]] || printf '{\n}\n' >"$settings_path" + # settings.json is JSONC: comments and trailing commas are legal there and + # common in practice, so it's edited in place rather than round-tripped + # through a strict JSON parser that would reject it and reformat the rest. + local escaped=${theme_name//&/\\&} + escaped=${escaped//|/\\|} + if ! grep -q '"workbench.colorTheme"' "$settings_path"; then sed -i --follow-symlinks -E '0,/\{/{s/\{/{\ "workbench.colorTheme": "",/}' "$settings_path" fi sed -i --follow-symlinks -E \ - "s/(\"workbench.colorTheme\"[[:space:]]*:[[:space:]]*\")[^\"]*(\")/\1$theme_name\2/" \ + "s|(\"workbench.colorTheme\"[[:space:]]*:[[:space:]]*\")[^\"]*(\")|\1$escaped\2|" \ "$settings_path" elif [[ -f $settings_path ]]; then sed -i --follow-symlinks -E 's/\"workbench\.colorTheme\"[[:space:]]*:[^,}]*,?//' "$settings_path" diff --git a/etc/sudoers.d/omarchy-tzupdate b/etc/sudoers.d/omarchy-tzupdate index 913fa174..d35adb82 100644 --- a/etc/sudoers.d/omarchy-tzupdate +++ b/etc/sudoers.d/omarchy-tzupdate @@ -1 +1 @@ -%wheel ALL=(root) NOPASSWD: /usr/bin/tzupdate, /usr/bin/timedatectl set-timezone * +%wheel ALL=(root) NOPASSWD: /usr/bin/timedatectl set-timezone * diff --git a/test/shell.d/timezone-test.sh b/test/shell.d/timezone-test.sh index 3a03c65f..c1880a14 100644 --- a/test/shell.d/timezone-test.sh +++ b/test/shell.d/timezone-test.sh @@ -7,9 +7,12 @@ source "$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)/base-test.sh" timezone_menu="$ROOT/bin/omarchy-menu-timezone" sudoers_file="$ROOT/etc/sudoers.d/omarchy-tzupdate" -grep -F '%wheel ALL=(root) NOPASSWD: /usr/bin/tzupdate, /usr/bin/timedatectl set-timezone *' "$sudoers_file" >/dev/null || +grep -F '%wheel ALL=(root) NOPASSWD: /usr/bin/timedatectl set-timezone *' "$sudoers_file" >/dev/null || fail "timezone sudoers rule allows passwordless timedatectl timezone changes" +! grep -F 'tzupdate' "$sudoers_file" >/dev/null || + fail "timezone sudoers rule does not grant passwordless tzupdate" + grep -F 'sudo timedatectl set-timezone "$timezone"' "$timezone_menu" >/dev/null || fail "timezone menu uses the passwordless sudoers timedatectl rule"