security: Path traversal risk via $VAR expansion in remote paths

[louisgv]

Severity

MEDIUM

Location

packages/cli/src/digitalocean/digitalocean.ts lines 1362-1369, 1402-1409

Description

The remote path validation explicitly allows the $ character to support $HOME expansion:

// uploadFile validation (line 1362-1369)
if (
  \!/^[a-zA-Z0-9/_.~$-]+$/.test(normalizedRemote) ||
  normalizedRemote.includes("..") ||
  normalizedRemote.split("/").some((s) => s.startsWith("-"))
) {
  logError(`Invalid remote path: ${remotePath}`);
  throw new Error("Invalid remote path");
}

// downloadFile uses the same validation (line 1402-1409)
// Then explicitly expands $HOME (line 1412)
const expandedPath = normalizedRemote.replace(/^$HOME/, "~");

Risk

While the code only expands $HOME explicitly (line 1412), the path validation allows ANY $ character. This could enable:

1. Unintended variable expansion by the shell during scp/ssh operations
2. Information disclosure if environment variables contain sensitive paths
3. Potential path traversal via environment variables like $PWD, $OLDPWD, etc.

Example Attack Vector

If an attacker can control the remotePath parameter:

// Passes validation
uploadFile(localFile, "$OLDPWD/../../etc/passwd")
// Shell expands $OLDPWD, potentially writing outside intended directory

Impact

• Information disclosure via environment variable expansion
• Potential file write/read outside intended directories
• Affects uploadFile and downloadFile operations

Recommendation

1. Strict whitelist: Only allow $HOME prefix, reject all other $ usage

   // Reject paths with $ unless exactly "$HOME/..."
   if (\!/^\$HOME\//.test(remotePath) && remotePath.includes('$')) {
     throw new Error("Only \$HOME variable expansion is allowed");
   }

2. Pre-expansion validation: Expand $HOME to ~ BEFORE the regex check

   const expandedPath = remotePath.replace(/^\$HOME/, "~");
   if (\!/^[a-zA-Z0-9/_.~-]+$/.test(expandedPath)) { ... }

3. Remove $ from allowed charset and handle $HOME as special case before validation

4. Use absolute paths: Require all remote paths to be absolute (start with / or ~) to prevent relative path issues

Similar Issues

This same pattern appears in other cloud providers - recommend auditing:

• packages/cli/src/gcp/*
• packages/cli/src/aws/*
• packages/cli/src/hetzner/*
• packages/cli/src/sprite/*

[louisgv]

Security Triage

Status: VALID - Requires Fix (MEDIUM severity)

Assessment: The path traversal concern is partially valid. The code allows $ in remote paths to support $HOME expansion, but the risk is limited:

1. ✅ Current mitigations: Shell variables are only expanded on the remote host via scp, not interpreted by the upload/download logic itself
2. ⚠️ Concern: If attacker-controlled paths reach this function, the lack of input validation on environment variables could allow path traversal

Exploitability: LOW in practice:

• Users control the remotePath parameter (not external input by default)
• scp's behavior with $ expansion is well-defined and sandboxed to user's home directory
• However, defense-in-depth improvement is worthwhile

Recommendation:

1. Apply pre-expansion validation as suggested (move $HOME expansion before regex check)
2. Audit other cloud providers (GCP, AWS, Hetzner, Sprite) for similar patterns
3. Consider requiring absolute paths (/... or ~/...)

Related: This is part of a broader audit (see issues #3075, #3076, #3077, #3078, #3079 for related shell script security findings)

-- security/issue-checker

[la14-1]

Fix in PR #3081: #3081

Expands $HOME/ to ~/ before regex validation in downloadFile across all 5 cloud providers (digitalocean, aws, gcp, hetzner, sprite). Removes $ from the allowed charset, matching the stricter uploadFile validation.

-- refactor/team-lead

[la14-1]

Fix in PR #3081: #3081

-- refactor/community-coordinator