eBay MCP Server Environment Variable Injection via Crafted Prompts

# Root Cause Analysis: GHSA-97rm-xj73-33jh

## Summary

The ebay-mcp package (versions <= 1.7.2) contains an environment variable injection vulnerability in the `updateEnvFile` function located in `src/auth/oauth.ts`. This function is called by the `ebay_set_user_tokens` and `ebay_set_user_tokens_with_expiry` MCP tools to persist OAuth tokens to the `.env` file. The function blindly wraps token values in double quotes without sanitizing newline characters (`\n`), allowing attackers to inject arbitrary environment variables into the configuration file. This could lead to OAuth flow hijacking, denial of service, or potentially remote code execution via controlled environment variables like `NODE_OPTIONS`.

## Impact

**Package:** ebay-mcp (npm)
**Affected Versions:** <= 1.7.2
**Severity:** HIGH (CVSS 8.3)

**Consequences:**
- **Configuration Overwrites:** Attackers can overwrite critical settings like `EBAY_REDIRECT_URI` to redirect OAuth callbacks to attacker-controlled endpoints
- **Denial of Service:** Injecting malformed configuration can prevent the server from starting
- **Potential RCE:** In environments where environment variables control execution (e.g., `NODE_OPTIONS`), attackers may achieve remote code execution

## Root Cause

The vulnerability exists in the `updateEnvFile` function which constructs `.env` file entries using string interpolation:

```typescript
const newLine = `${key}="${value}"`;
```

This approach fails to:
1. Escape newline characters (`\n`) within the value
2. Validate or sanitize user-controlled input before writing to the file system

When a token value containing a newline is processed, the newline character terminates the quoted string early and begins a new line in the file. For example, a token value of:
```
v1.MTIzNDU2Nzg5MA==\nATTACK_VAR=malicious_value
```

Results in the following `.env` content:
```
EBAY_USER_ACCESS_TOKEN="v1.MTIzNDU2Nzg5MA==
ATTACK_VAR=malicious_value"
```

This effectively injects `ATTACK_VAR=malicious_value` as a separate environment variable.

**Fix Commit:** https://github.com/YosefHayim/ebay-mcp/commit/aab0bda75ea9dd27aa37d0d8524d7cf41b3c4a9a

The fix replaces the manual string manipulation with proper parsing and serialization using the `dotenv` and `dotenv-stringify` libraries, which correctly handle special characters.

## Reproduction Steps

The reproduction script is located at `repro/reproduction_steps.sh`. It:

1. Creates a minimal Node.js environment with the vulnerable `updateEnvFile` function
2. Sets up an initial `.env` file with legitimate configuration
3. Simulates calling the function with malicious tokens containing newlines
4. Parses the resulting `.env` file to verify arbitrary variable injection

**Expected Evidence:**
The script should output confirmation that environment variables `ATTACK_VAR` and `SECOND_ATTACK` were successfully injected into the `.env` file.

## Evidence

**Log Location:** `logs/reproduction_output.log`

**Key Excerpts:**

```
=== Resulting .env file after injection ===
EBAY_APP_ID=test_app
EBAY_CERT_ID=test_cert
EBAY_REDIRECT_URI=https://example.com/callback

EBAY_USER_ACCESS_TOKEN="v1.MTIzNDU2Nzg5MA==
ATTACK_VAR=malicious_value_injected"
EBAY_USER_REFRESH_TOKEN="v1.AbCdEfGhIjKl
SECOND_ATTACK=second_payload"

=== Vulnerability Analysis ===
❌ VULNERABILITY CONFIRMED: Environment variable injection successful!
Injected variables:
  - ATTACK_VAR=malicious_value_injected"
  - SECOND_ATTACK=second_payload"
```

**Environment Details:**
- Tested with Node.js (via shell script using ES modules)
- ebay-mcp version <= 1.7.2 (vulnerable code extracted from source)

## Recommendations / Next Steps

**Immediate Actions:**
1. Upgrade ebay-mcp to the patched version (commit aab0bda or later)
2. Audit existing `.env` files for injected malicious variables if the server has been running in untrusted environments

**Long-term Security Improvements:**
1. Implement input validation on all user-controlled data before file system operations
2. Use established libraries (like dotenv with proper serialization) rather than custom string manipulation for configuration files
3. Consider using temporary files and atomic moves to prevent partial writes
4. Add security unit tests specifically for injection vectors (newlines, quotes, null bytes)

**Testing Recommendations:**
- Add regression test with malicious token payloads containing: `\n`, `\r`, `"`, `'`, `$`, backticks
- Verify that the dotenv-stringify library properly escapes these characters
- Test with Unicode and multi-byte characters

## Additional Notes

**Idempotency Confirmation:** The reproduction script was run twice consecutively with identical results, confirming reproducibility.

**Edge Cases:**
- The vulnerability also affects the `refreshUserAccessToken` method which calls `updateEnvFile` with refreshed tokens from eBay's API
- If an attacker compromises the eBay API or performs a man-in-the-middle attack, they could inject malicious tokens at the source
- The `.env` file permissions should be restrictive (600) to prevent other users from reading injected secrets

**Related CWEs:**
- CWE-15: External Control of System or Configuration Setting
- CWE-74: Improper Neutralization of Special Elements in Output Used by a Downstream Component ('Injection')