From 290c4c13801a0e3c04addbad04416207ccf46513 Mon Sep 17 00:00:00 2001 From: Stephen Braverman Date: Mon, 23 Mar 2026 14:39:55 -0400 Subject: [PATCH 1/4] [PPSC-602] fix: validate install directory against allowed paths (CWE-22) Co-Authored-By: Claude Opus 4.6 --- scripts/install.ps1 | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/scripts/install.ps1 b/scripts/install.ps1 index 0165c21..823ac87 100644 --- a/scripts/install.ps1 +++ b/scripts/install.ps1 @@ -171,6 +171,24 @@ function Main { exit 1 } + # Validate install directory is under user-writable locations + $allowedPrefixes = @($env:LOCALAPPDATA, $env:APPDATA, $env:USERPROFILE, $env:ProgramFiles) + $isAllowed = $false + foreach ($prefix in $allowedPrefixes) { + if ($prefix -and $InstallDir.StartsWith($prefix, [System.StringComparison]::OrdinalIgnoreCase)) { + $isAllowed = $true + break + } + } + if (-not $isAllowed) { + Write-Warning "Install directory '$InstallDir' is outside standard user locations." + $confirm = Read-Host "Continue? (y/N)" + if ($confirm -ne 'y' -and $confirm -ne 'Y') { + Write-Error "Installation cancelled by user" + exit 1 + } + } + # Validate Version format (except for the special 'latest' value) if ($Version -ne "latest" -and $Version -notmatch '^v?\d+\.\d+\.\d+(-[0-9A-Za-z\.-]+)?$') { Write-Error "Invalid version format: '$Version'. Expected formats like 'v1.2.3' or '1.2.3' (optionally with suffix like -beta.1), or 'latest'." From 60de804e4d7e992e5cc1eb16fd0c533bce9404b8 Mon Sep 17 00:00:00 2001 From: Yiftach Cohen Date: Wed, 25 Mar 2026 13:25:18 +0200 Subject: [PATCH 2/4] [PPSC-602] fix: harden install path validation (CWE-22) - Remove Program Files from allowed prefixes to prevent DLL planting - Normalize prefixes with trailing backslash to avoid prefix confusion - Detect non-interactive mode and fail fast instead of hanging on Read-Host - Add cross-reference comments between install.sh and install.ps1 - Fix shellcheck warnings (redundant patterns, declare/assign separation) --- scripts/install.ps1 | 30 +++++++++++++-- scripts/install.sh | 89 ++++++++++++++++++++++++--------------------- 2 files changed, 74 insertions(+), 45 deletions(-) diff --git a/scripts/install.ps1 b/scripts/install.ps1 index 823ac87..1862df3 100644 --- a/scripts/install.ps1 +++ b/scripts/install.ps1 @@ -171,16 +171,40 @@ function Main { exit 1 } - # Validate install directory is under user-writable locations - $allowedPrefixes = @($env:LOCALAPPDATA, $env:APPDATA, $env:USERPROFILE, $env:ProgramFiles) + # Validate install directory is under standard user-profile locations. + # Program Files is intentionally excluded — it requires admin privileges and + # writing there could enable DLL/EXE planting. Non-standard paths are allowed + # only with explicit interactive confirmation. + # Note: install.sh uses a different model (character-set + traversal deny-list) + # because Unix paths don't have canonical env-var-based user directories. + $allowedPrefixes = @($env:LOCALAPPDATA, $env:APPDATA, $env:USERPROFILE) $isAllowed = $false foreach ($prefix in $allowedPrefixes) { - if ($prefix -and $InstallDir.StartsWith($prefix, [System.StringComparison]::OrdinalIgnoreCase)) { + if (-not $prefix) { continue } + # Normalize prefix the same way $InstallDir was normalized above, + # then append trailing backslash to prevent prefix confusion + # (e.g., "LocalLow" matching "Local") + $prefix = [System.IO.Path]::GetFullPath($prefix) + $normalizedPrefix = $prefix.TrimEnd('\') + '\' + if ($InstallDir.StartsWith($normalizedPrefix, [System.StringComparison]::OrdinalIgnoreCase)) { $isAllowed = $true break } } if (-not $isAllowed) { + # In non-interactive contexts (e.g., irm ... | iex), Read-Host would hang + # or return empty. Detect this and fail immediately instead. + # Wrap in try/catch because [Console]::IsInputRedirected can throw in + # some hosts (ISE, remote sessions) where console handles are unavailable. + try { + $isInteractive = [Environment]::UserInteractive -and -not [Console]::IsInputRedirected + } catch { + $isInteractive = $false + } + if (-not $isInteractive) { + Write-Error "Install directory '$InstallDir' is outside standard user locations and cannot be confirmed in non-interactive mode. Use -InstallDir with a path under `$env:LOCALAPPDATA, `$env:APPDATA, or `$env:USERPROFILE." + exit 1 + } Write-Warning "Install directory '$InstallDir' is outside standard user locations." $confirm = Read-Host "Continue? (y/N)" if ($confirm -ne 'y' -and $confirm -ne 'Y') { diff --git a/scripts/install.sh b/scripts/install.sh index 7d8d440..f00b343 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -20,6 +20,9 @@ validate_version() { fi } +# Validates install directory safety using character-set + traversal deny-list. +# Note: install.ps1 uses a different model (user-profile allowlist) because +# Windows has canonical env-var-based user directories (LOCALAPPDATA, APPDATA). validate_install_dir() { local dir="$1" @@ -47,7 +50,7 @@ validate_install_dir() { # Disallow parent directory traversal segments like "../" or "/../" or "/.." # This allows valid paths with ".." in filenames (e.g., /opt/app..v1/bin) case "$dir" in - */../*|../*|*/..|/..) + */../*|../*|*/..) echo "Error: Install directory cannot contain parent directory segment '..': $dir" >&2 exit 1 ;; @@ -71,7 +74,7 @@ validate_install_dir() { # Disallow parent directory traversal segments in the normalized path case "$normalized_dir" in - */../*|../*|*/..|/..) + */../*|../*|*/..) echo "Error: Normalized install directory cannot contain parent directory segment '..': $normalized_dir" >&2 exit 1 ;; @@ -105,7 +108,7 @@ detect_arch() { download_file() { local url="$1" local output="$2" - + if command -v curl > /dev/null 2>&1; then curl -fsSL "$url" -o "$output" elif command -v wget > /dev/null 2>&1; then @@ -131,16 +134,17 @@ is_in_path() { add_to_path() { local dir="$1" - local shell_name=$(basename "$SHELL") + local shell_name + shell_name=$(basename "$SHELL") local rc_file="" local path_line="export PATH=\"$dir:\$PATH\"" - + # Skip in CI/CD environments if is_ci_environment; then echo "ℹ️ CI/CD environment detected, skipping PATH modification" return 0 fi - + # Detect shell config file case "$shell_name" in zsh) @@ -163,25 +167,25 @@ add_to_path() { return 1 ;; esac - + # Check if already in config file if [ -f "$rc_file" ] && grep -q "$dir" "$rc_file" 2>/dev/null; then echo "ℹ️ PATH already configured in $rc_file" return 0 fi - + # Add to config file echo "" echo "📝 Adding $dir to PATH in $rc_file..." - + # Create config file directory if it doesn't exist (for fish) mkdir -p "$(dirname "$rc_file")" 2>/dev/null || true - + echo "$path_line" >> "$rc_file" || { echo "❌ Failed to update $rc_file" return 1 } - + echo "✅ PATH updated in $rc_file" return 0 } @@ -221,21 +225,21 @@ choose_install_dir() { echo "$INSTALL_DIR" return fi - + # Strategy: Prefer directories already in PATH to avoid shell restart - + # 1. Check if /usr/local/bin is writable (common on macOS with Homebrew) if [ -d "$SYSTEM_BIN" ] && [ -w "$SYSTEM_BIN" ]; then echo "$SYSTEM_BIN" return fi - + # 2. Check if ~/.local/bin exists and is already in PATH if [ -d "$USER_BIN" ] && is_in_path "$USER_BIN"; then echo "$USER_BIN" return fi - + # 3. Try to create ~/.local/bin if it doesn't exist if [ ! -d "$USER_BIN" ] && mkdir -p "$USER_BIN" 2>/dev/null; then if is_in_path "$USER_BIN"; then @@ -243,7 +247,7 @@ choose_install_dir() { return fi fi - + # 4. Fall back to /usr/local/bin (will require sudo if not writable) echo "$SYSTEM_BIN" } @@ -252,12 +256,12 @@ verify_checksums() { local archive_file="$1" local checksums_file="$2" local checksums_sig="$3" - + if [ "$VERIFY" != "true" ]; then echo "⚠️ Skipping verification (VERIFY=false)" return 0 fi - + if command -v cosign > /dev/null 2>&1; then echo "🔐 Verifying signature with cosign..." if cosign verify-blob \ @@ -273,17 +277,17 @@ verify_checksums() { echo "ℹ️ cosign not found, verifying checksums only" echo " Install cosign for full signature verification: https://docs.sigstore.dev/cosign/installation/" fi - + echo "🔍 Verifying checksums..." - + local expected_checksum expected_checksum=$(awk -v filename="$(basename "$archive_file")" '$2 == filename {print $1}' "$checksums_file") - + if [ -z "$expected_checksum" ]; then echo "⚠️ Could not find checksum for $(basename "$archive_file") in checksums file" return 1 fi - + local actual_checksum if command -v sha256sum > /dev/null 2>&1 && sha256sum --version > /dev/null 2>&1; then actual_checksum=$(sha256sum "$archive_file" | awk '{print $1}') @@ -294,14 +298,14 @@ verify_checksums() { echo " Install sha256sum or shasum and try again." return 1 fi - + if [ "$expected_checksum" != "$actual_checksum" ]; then echo "❌ Checksum verification failed!" echo " Expected: $expected_checksum" echo " Got: $actual_checksum" return 1 fi - + echo "✓ Checksums verified successfully" } @@ -332,46 +336,46 @@ main() { else BASE_URL="https://github.com/${REPO}/releases/download/${VERSION}" fi - + ARCHIVE_NAME="${BINARY_NAME}-${OS}-${ARCH}.tar.gz" if [ "$OS" = "windows" ]; then ARCHIVE_NAME="${BINARY_NAME}-${OS}-${ARCH}.zip" fi - + TMP_DIR=$(mktemp -d) || { echo "Error: Failed to create temporary directory" >&2; exit 1; } trap 'rm -rf "$TMP_DIR"' EXIT - + ARCHIVE_FILE="$TMP_DIR/$ARCHIVE_NAME" CHECKSUMS_FILE="$TMP_DIR/${BINARY_NAME}-checksums.txt" CHECKSUMS_SIG="$TMP_DIR/${BINARY_NAME}-checksums.txt.sig" - + echo "📦 Downloading $ARCHIVE_NAME..." download_file "$BASE_URL/$ARCHIVE_NAME" "$ARCHIVE_FILE" - + echo "📥 Downloading checksums..." download_file "$BASE_URL/${BINARY_NAME}-checksums.txt" "$CHECKSUMS_FILE" download_file "$BASE_URL/${BINARY_NAME}-checksums.txt.sig" "$CHECKSUMS_SIG" || true - + echo "" verify_checksums "$ARCHIVE_FILE" "$CHECKSUMS_FILE" "$CHECKSUMS_SIG" echo "" - + echo "📂 Extracting archive..." if [ "$OS" = "windows" ]; then unzip -q "$ARCHIVE_FILE" -d "$TMP_DIR" else tar -xzf "$ARCHIVE_FILE" -C "$TMP_DIR" fi - + BINARY_FILE="$TMP_DIR/$BINARY_NAME" if [ "$OS" = "windows" ]; then BINARY_FILE="${BINARY_FILE}.exe" fi - + chmod +x "$BINARY_FILE" TARGET_PATH="$INSTALL_DIR/$BINARY_NAME" - + # Check if upgrading existing installation EXISTING_VERSION="" if [ -f "$TARGET_PATH" ]; then @@ -385,7 +389,7 @@ main() { else echo "📦 Installing to $INSTALL_DIR..." fi - + if [ -w "$INSTALL_DIR" ]; then mv "$BINARY_FILE" "$TARGET_PATH" || fail "Failed to move binary to $TARGET_PATH" else @@ -393,27 +397,27 @@ main() { sudo -v || fail "sudo authentication failed" sudo mv "$BINARY_FILE" "$TARGET_PATH" || fail "Failed to move binary to $TARGET_PATH (sudo mv failed)" fi - + if [ -w "$TARGET_PATH" ]; then chmod +x "$TARGET_PATH" 2>/dev/null || true else sudo chmod +x "$TARGET_PATH" 2>/dev/null || true fi - + [ -f "$TARGET_PATH" ] || fail "Install appeared to succeed, but $TARGET_PATH does not exist" # Get installed version INSTALLED_VERSION=$("$TARGET_PATH" --version 2>/dev/null | head -n1 || echo "unknown") - + echo "" echo "✅ Armis CLI installed successfully!" echo " Location: $TARGET_PATH" echo " Version: $INSTALLED_VERSION" echo "" - + # Refresh command hash table for current shell hash -r 2>/dev/null || rehash 2>/dev/null || true - + # Check if command is now available if command -v "$BINARY_NAME" >/dev/null 2>&1; then # Success! Command is in PATH and discoverable @@ -434,14 +438,15 @@ main() { # Command not in PATH yet echo "⚠️ Installation complete, but '$BINARY_NAME' is not in your PATH yet." echo "" - + if ! is_in_path "$INSTALL_DIR"; then # Need to add to PATH if add_to_path "$INSTALL_DIR"; then echo "" echo "📋 To use $BINARY_NAME, you need to reload your shell configuration:" echo "" - local shell_name=$(basename "$SHELL") + local shell_name + shell_name=$(basename "$SHELL") case "$shell_name" in zsh) echo " source ~/.zshrc" From eb01e0b58766b1faa159389d23f4efac3a3fc220 Mon Sep 17 00:00:00 2001 From: Yiftach Cohen Date: Wed, 25 Mar 2026 13:39:51 +0200 Subject: [PATCH 3/4] [PPSC-602] fix: address CWE-269 privilege escalation and CWE-78 command injection - CWE-269: Refuse sudo escalation when INSTALL_DIR is set via env var to a non-writable directory. Only the auto-detected fallback may use sudo. - CWE-78: Remove pre-existing binary execution for version detection in both install.sh and install.ps1. An attacker could plant a malicious binary at the install path before the installer runs. --- scripts/install.ps1 | 15 +++------------ scripts/install.sh | 22 +++++++++++++--------- 2 files changed, 16 insertions(+), 21 deletions(-) diff --git a/scripts/install.ps1 b/scripts/install.ps1 index 1862df3..ab3763a 100644 --- a/scripts/install.ps1 +++ b/scripts/install.ps1 @@ -269,19 +269,10 @@ function Main { $binarySource = Join-Path $tmpDir $BinaryName $binaryDest = Join-Path $InstallDir $BinaryName - # Detect upgrade vs fresh install + # CWE-78: Do NOT execute the pre-existing binary for version detection — + # an attacker could have planted a malicious executable at this path. if (Test-Path $binaryDest) { - try { - $existingVersion = & $binaryDest --version 2>$null | Select-Object -First 1 - if ($existingVersion) { - Write-Host "Upgrading existing installation..." - Write-Host " Current: $existingVersion" - } else { - Write-Host "Replacing existing installation..." - } - } catch { - Write-Host "Replacing existing installation..." - } + Write-Host "Replacing existing installation..." } Copy-Item -Path $binarySource -Destination $binaryDest -Force diff --git a/scripts/install.sh b/scripts/install.sh index f00b343..083c210 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -222,6 +222,9 @@ choose_install_dir() { # Allow override via environment variable if [ -n "${INSTALL_DIR:-}" ]; then validate_install_dir "$INSTALL_DIR" + # Flag that the directory came from the env var so the installer + # can refuse sudo escalation for user-supplied paths (CWE-269). + INSTALL_DIR_FROM_ENV=1 echo "$INSTALL_DIR" return fi @@ -376,16 +379,11 @@ main() { TARGET_PATH="$INSTALL_DIR/$BINARY_NAME" - # Check if upgrading existing installation - EXISTING_VERSION="" + # Check if upgrading existing installation. + # CWE-78: Do NOT execute the pre-existing binary for version detection — + # an attacker could have planted a malicious binary at this path. if [ -f "$TARGET_PATH" ]; then - EXISTING_VERSION=$("$TARGET_PATH" --version 2>/dev/null | head -n1 || echo "") - if [ -n "$EXISTING_VERSION" ]; then - echo "📦 Upgrading existing installation..." - echo " Current: $EXISTING_VERSION" - else - echo "📦 Replacing existing installation..." - fi + echo "📦 Replacing existing installation..." else echo "📦 Installing to $INSTALL_DIR..." fi @@ -393,6 +391,12 @@ main() { if [ -w "$INSTALL_DIR" ]; then mv "$BINARY_FILE" "$TARGET_PATH" || fail "Failed to move binary to $TARGET_PATH" else + # CWE-269: If the user explicitly set INSTALL_DIR via env var to a + # directory they cannot write to, refuse to escalate to sudo. + # Only the auto-detected fallback (/usr/local/bin) may use sudo. + if [ -n "${INSTALL_DIR_FROM_ENV:-}" ]; then + fail "Install directory '$INSTALL_DIR' is not writable and was set via INSTALL_DIR env var. Choose a directory you have write access to, or unset INSTALL_DIR to use the default." + fi echo " (requires sudo privileges)" sudo -v || fail "sudo authentication failed" sudo mv "$BINARY_FILE" "$TARGET_PATH" || fail "Failed to move binary to $TARGET_PATH (sudo mv failed)" From 6c0376be6ff1f0204771ce3c7a40ff55aa58f95f Mon Sep 17 00:00:00 2001 From: Yiftach Cohen Date: Wed, 25 Mar 2026 16:35:06 +0200 Subject: [PATCH 4/4] [PPSC-602] fix: address CWE-426 untrusted search path and CWE-78 post-install execution - CWE-426: Only add install directory to PATH for allowlisted locations (LOCALAPPDATA, APPDATA, USERPROFILE). Non-standard paths confirmed interactively are not added to PATH automatically, preventing an attacker-influenced directory from polluting the search path. - CWE-78: Only execute the installed binary for version display when integrity verification was performed (checksums validated). Without verification, the binary content cannot be trusted. --- scripts/install.ps1 | 63 +++++++++++++++++++++++++++------------------ 1 file changed, 38 insertions(+), 25 deletions(-) diff --git a/scripts/install.ps1 b/scripts/install.ps1 index ab3763a..7cd7d87 100644 --- a/scripts/install.ps1 +++ b/scripts/install.ps1 @@ -178,7 +178,7 @@ function Main { # Note: install.sh uses a different model (character-set + traversal deny-list) # because Unix paths don't have canonical env-var-based user directories. $allowedPrefixes = @($env:LOCALAPPDATA, $env:APPDATA, $env:USERPROFILE) - $isAllowed = $false + $isAllowlisted = $false foreach ($prefix in $allowedPrefixes) { if (-not $prefix) { continue } # Normalize prefix the same way $InstallDir was normalized above, @@ -187,11 +187,11 @@ function Main { $prefix = [System.IO.Path]::GetFullPath($prefix) $normalizedPrefix = $prefix.TrimEnd('\') + '\' if ($InstallDir.StartsWith($normalizedPrefix, [System.StringComparison]::OrdinalIgnoreCase)) { - $isAllowed = $true + $isAllowlisted = $true break } } - if (-not $isAllowed) { + if (-not $isAllowlisted) { # In non-interactive contexts (e.g., irm ... | iex), Read-Host would hang # or return empty. Detect this and fail immediately instead. # Wrap in try/catch because [Console]::IsInputRedirected can throw in @@ -277,36 +277,49 @@ function Main { Copy-Item -Path $binarySource -Destination $binaryDest -Force - # Add to PATH (skip persistent changes in CI environments where PATH is ephemeral) - if (-not (Test-CIEnvironment)) { - $currentPath = [Environment]::GetEnvironmentVariable("Path", "User") - $newUserPath = Add-DirectoryToPath -ExistingPath $currentPath -Directory $InstallDir - if ($newUserPath -ne $currentPath) { - Write-Host "Adding to PATH..." - [Environment]::SetEnvironmentVariable( - "Path", - $newUserPath, - "User" - ) + # CWE-426: Only modify PATH for allowlisted directories. Non-standard + # paths (confirmed interactively) require the user to add them manually, + # preventing an attacker-influenced directory from being added to the + # search path. + if ($isAllowlisted) { + # Add to PATH (skip persistent changes in CI environments where PATH is ephemeral) + if (-not (Test-CIEnvironment)) { + $currentPath = [Environment]::GetEnvironmentVariable("Path", "User") + $newUserPath = Add-DirectoryToPath -ExistingPath $currentPath -Directory $InstallDir + if ($newUserPath -ne $currentPath) { + Write-Host "Adding to PATH..." + [Environment]::SetEnvironmentVariable( + "Path", + $newUserPath, + "User" + ) + $env:Path = Add-DirectoryToPath -ExistingPath $env:Path -Directory $InstallDir + Write-Host "Added $InstallDir to user PATH" -ForegroundColor Green + Write-Host " (Restart your terminal for PATH changes to take effect)" + } + } else { $env:Path = Add-DirectoryToPath -ExistingPath $env:Path -Directory $InstallDir - Write-Host "Added $InstallDir to user PATH" -ForegroundColor Green - Write-Host " (Restart your terminal for PATH changes to take effect)" } } else { - $env:Path = Add-DirectoryToPath -ExistingPath $env:Path -Directory $InstallDir + Write-Host " Note: '$InstallDir' was not added to PATH automatically (non-standard location)." + Write-Host " Add it manually if needed: `$env:Path += `";$InstallDir`"" } Write-Host "" Write-Host "Armis CLI installed successfully!" -ForegroundColor Green - # Show installed version - try { - $newVersion = & $binaryDest --version 2>$null | Select-Object -First 1 - if ($newVersion) { - Write-Host " Location: $binaryDest" - Write-Host " Version: $newVersion" - } - } catch {} + Write-Host " Location: $binaryDest" + # CWE-78: Only execute the installed binary for version display when + # integrity verification was performed (checksums validated). Without + # verification, we cannot trust the binary content. + if ($Verify) { + try { + $newVersion = & $binaryDest --version 2>$null | Select-Object -First 1 + if ($newVersion) { + Write-Host " Version: $newVersion" + } + } catch {} + } Write-Host "" Write-Host "Run 'armis-cli --help' to get started"