1
0
mirror of https://github.com/tw93/Mole.git synced 2026-02-04 11:31:46 +00:00

fix: enhance uninstall security per audit review

- Validate bundle_id format (reverse-DNS) in stop_launch_services() to prevent glob injection attacks
    - Add common word exclusion list for LaunchAgents name search to avoid false positive matches (Music, Notes, Photos, etc.) - Add security comments explaining symlink handling in remove_file_list()
    - Improve brew_uninstall_cask() timeout handling: exit code 124 now returns failure immediately
    - Update SECURITY_AUDIT.md with remediation details
This commit is contained in:
tw93
2026-01-26 20:27:46 +08:00
parent 09f0cf9e53
commit 3919a70300
4 changed files with 46 additions and 8 deletions

View File

@@ -768,14 +768,23 @@ find_app_files() {
# Note: LaunchDaemons are system-level and handled in find_app_system_files()
# Minimum 5-char threshold prevents false positives (e.g., "Time" matching system agents)
# Short-name apps (e.g., Zoom, Arc) are still cleaned via bundle_id matching above
# Security: Common words are excluded to prevent matching unrelated plist files
if [[ ${#app_name} -ge 5 ]] && [[ -d ~/Library/LaunchAgents ]]; then
while IFS= read -r -d '' plist; do
local plist_name=$(basename "$plist")
if [[ "$plist_name" =~ ^com\.apple\. ]]; then
continue
fi
files_to_clean+=("$plist")
done < <(command find ~/Library/LaunchAgents -maxdepth 1 -name "*$app_name*.plist" -print0 2> /dev/null)
# Skip common words that could match many unrelated LaunchAgents
# These are either generic terms or names that overlap with system/common utilities
local common_words="Music|Notes|Photos|Finder|Safari|Preview|Calendar|Contacts|Messages|Reminders|Clock|Weather|Stocks|Books|News|Podcasts|Voice|Files|Store|System|Helper|Agent|Daemon|Service|Update|Sync|Backup|Cloud|Manager|Monitor|Server|Client|Worker|Runner|Launcher|Driver|Plugin|Extension|Widget|Utility"
if [[ "$app_name" =~ ^($common_words)$ ]]; then
debug_log "Skipping LaunchAgent name search for common word: $app_name"
else
while IFS= read -r -d '' plist; do
local plist_name=$(basename "$plist")
# Skip Apple's LaunchAgents
if [[ "$plist_name" =~ ^com\.apple\. ]]; then
continue
fi
files_to_clean+=("$plist")
done < <(command find ~/Library/LaunchAgents -maxdepth 1 -name "*$app_name*.plist" -print0 2> /dev/null)
fi
fi
# Handle specialized toolchains and development environments

View File

@@ -72,12 +72,20 @@ decode_file_list() {
# Note: find_app_files() and calculate_total_size() are in lib/core/common.sh.
# Stop Launch Agents/Daemons for an app.
# Security: bundle_id is validated to be reverse-DNS format before use in find patterns
stop_launch_services() {
local bundle_id="$1"
local has_system_files="${2:-false}"
[[ -z "$bundle_id" || "$bundle_id" == "unknown" ]] && return 0
# Validate bundle_id format: must be reverse-DNS style (e.g., com.example.app)
# This prevents glob injection attacks if bundle_id contains special characters
if [[ ! "$bundle_id" =~ ^[a-zA-Z0-9][-a-zA-Z0-9]*(\.[a-zA-Z0-9][-a-zA-Z0-9]*)+$ ]]; then
debug_log "Invalid bundle_id format for LaunchAgent search: $bundle_id"
return 0
fi
if [[ -d ~/Library/LaunchAgents ]]; then
while IFS= read -r -d '' plist; do
launchctl unload "$plist" 2> /dev/null || true
@@ -135,6 +143,7 @@ remove_login_item() {
}
# Remove files (handles symlinks, optional sudo).
# Security: All paths pass validate_path_for_deletion() before any deletion.
remove_file_list() {
local file_list="$1"
local use_sudo="${2:-false}"
@@ -147,6 +156,12 @@ remove_file_list() {
continue
fi
# Symlinks are handled separately using rm (not safe_remove/safe_sudo_remove)
# because safe_sudo_remove() refuses symlinks entirely as a TOCTOU protection.
# This is safe because:
# 1. The path has already passed validate_path_for_deletion() above
# 2. rm on a symlink only removes the link itself, NOT the target
# 3. The symlink deletion is logged via operations.log
if [[ -L "$file" ]]; then
if [[ "$use_sudo" == "true" ]]; then
sudo rm "$file" 2> /dev/null && ((++count)) || true

View File

@@ -196,15 +196,22 @@ brew_uninstall_cask() {
fi
# Run with timeout to prevent hangs from problematic cask scripts
local brew_exit=0
if HOMEBREW_NO_ENV_HINTS=1 HOMEBREW_NO_AUTO_UPDATE=1 NONINTERACTIVE=1 \
run_with_timeout "$timeout" brew uninstall --cask "$cask_name" 2>&1; then
uninstall_ok=true
else
brew_exit=$?
debug_log "brew uninstall timeout or failed with exit code: $brew_exit"
# Exit code 124 indicates timeout from run_with_timeout
# On timeout, fail immediately without verification to avoid inconsistent state
if [[ $brew_exit -eq 124 ]]; then
debug_log "brew uninstall timed out after ${timeout}s, returning failure"
return 1
fi
fi
# Verify removal
# Verify removal (only if not timed out)
local cask_gone=true app_gone=true
HOMEBREW_NO_ENV_HINTS=1 brew list --cask 2> /dev/null | grep -qxF "$cask_name" && cask_gone=false
[[ -n "$app_path" && -e "$app_path" ]] && app_gone=false