# Patch Analysis — CVE-2026-58169 (Vibe-Trading DNS Rebinding Auth Bypass → RCE)

## Overview

The vulnerability (CVE-2026-58169) is a DNS-rebinding authentication bypass in
`HKUDS/Vibe-Trading` (<0.1.10). The FastAPI API server (`agent/api_server.py`)
binds to `0.0.0.0` by default and treats any TCP connection whose peer address
is loopback as a trusted local/dev-mode caller, skipping `API_AUTH_KEY` bearer
authentication. Because the vulnerable versions performed **no Host-header
validation**, DNS rebinding lets an attacker-controlled domain resolve to
`127.0.0.1`; the victim's browser then issues a same-origin JSON request to the
local API carrying the attacker's `Host` header and no bearer token. The
loopback peer IP causes `_is_local_client()` → `True`, so `_validate_api_auth()`
returns immediately, and `_shell_tools_enabled_for_request()` → `True`, enabling
the `bash` tool. The agent is then driven (via a poisoned LLM endpoint or a
crafted user message) to invoke `BashTool`, which runs
`subprocess.run(command, shell=True)` — arbitrary RCE as the API process user.

The v0.1.10 release ships **three coordinated security fixes**:

| PR | Commit | Change |
|----|--------|--------|
| #242 | `6e06017` | `_reject_untrusted_loopback_host` HTTP middleware: reject loopback requests whose `Host` is not in the allowlist (`HTTP 403 "Untrusted local API host"`). |
| #243 | `7eb8dea` | `_shell_tools_enabled_for_request()` no longer infers privilege from peer IP; shell tools (`bash`, `background_run`) require explicit `VIBE_TRADING_ENABLE_SHELL_TOOLS` opt-in. |
| #245 | `b608fc5` | `require_settings_write_auth`: credential-routing settings writes (`PUT /settings/llm`, `PUT /settings/data-sources`) require a bearer token even for loopback when `API_AUTH_KEY` is configured. |

## What the fix changes (files / functions / logic)

### `agent/api_server.py`

**1. Host allowlist + middleware (PR #242)**

```python
_DEFAULT_LOOPBACK_HOSTS = frozenset({
    "localhost", "127.0.0.1", "::1", "[::1]",
    "testserver",   # <-- Starlette TestClient default; test convenience
})

_EXTRA_LOOPBACK_HOSTS = _parse_extra_loopback_hosts(os.getenv("API_ALLOWED_HOSTS"))

def _host_without_port(host: str) -> str: ...      # lowercase, strip port, keep [::1] brackets
def _is_allowed_loopback_host(host: str) -> bool:
    normalized = _host_without_port(host)
    return normalized in _DEFAULT_LOOPBACK_HOSTS or normalized in _EXTRA_LOOPBACK_HOSTS

@app.middleware("http")
async def _reject_untrusted_loopback_host(request, call_next):
    if _is_local_client(request) and not _is_allowed_loopback_host(request.headers.get("host", "")):
        return JSONResponse(status_code=403, content={"detail": "Untrusted local API host"})
    return await call_next(request)
```

The middleware runs for every HTTP request **before** route handlers. For a
loopback peer, it requires the `Host` header (port stripped) to be one of the
allowlisted names. Non-loopback peers pass through and are governed by
per-route bearer auth (`_validate_api_auth` raises 403 for non-local when
`API_AUTH_KEY` is unset).

**2. Shell-tool gating (PR #243)**

```python
# v0.1.9 (vulnerable):   return _is_local_client(request) or _env_shell_tools_enabled()
# v0.1.10 (fixed):       return _env_shell_tools_enabled()
def _shell_tools_enabled_for_request(request: Request) -> bool:
    return _env_shell_tools_enabled()   # only VIBE_TRADING_ENABLE_SHELL_TOOLS
```

In `agent/src/tools/__init__.py`, `_SHELL_TOOL_NAMES = {"bash",
"background_run"}` are skipped during registry build unless
`include_shell_tools` is true. So even if the Host gate is bypassed, the bash
RCE primitive is off by default.

**3. Settings-write auth (PR #245)**

`require_settings_write_auth` requires a valid bearer token (when
`API_AUTH_KEY` is set) before `PUT /settings/llm` and `PUT /settings/data-sources`,
closing the credential-exfiltration / LLM-redirection path.

## What invariant the fix relies on

> *"A loopback peer can only carry a Host header that names a loopback address
> (or an operator-configured name). Any other Host on a loopback peer is a
> DNS-rebinding artifact and must be rejected."*

Secondary invariants (defense in depth):
- Shell-tool privilege is **never** inferred from peer IP (must be env-opted-in).
- Credential-routing settings writes require an explicit bearer token.

## What the fix does NOT cover (gaps found)

### Gap 1 (CONFIRMED, partial bypass): `testserver` in the production allowlist

`_DEFAULT_LOOPBACK_HOSTS` includes `"testserver"` — a **non-loopback** hostname
added purely so Starlette/FastAPI's `TestClient` (whose default Host is
`testserver`) can exercise the API in unit tests without overriding `Host` on
every request. This test-convenience entry leaked into the **production**
allowlist used by the security middleware.

Consequence: a loopback peer request carrying `Host: testserver:<port>` passes
`_is_allowed_loopback_host()` (`testserver` ∈ allowlist) and is therefore
**auth-bypassed on the FIXED v0.1.10**, exactly as a DNS-rebound attacker-domain
request would have been on v0.1.9. Demonstrated at runtime:

- Fixed v0.1.10, `POST /sessions`, `Host: testserver:18899`, no bearer token →
  **HTTP 201** (session created, auth bypassed).
- Fixed v0.1.10, `POST /sessions`, `Host: rebind.attacker.test:18899` →
  **HTTP 403** (the fix correctly blocks the standard DNS-rebinding Host).

So the Host-validation fix is **incomplete**: its allowlist trusts a name that
is not a loopback address and is not operator-configured.

**Exploitability caveat (honest assessment):** In the **standard DNS-rebinding
threat model**, the attacker cannot make the victim's browser send
`Host: testserver` — DNS rebinding requires the attacker to control an
*attacker-owned* domain (e.g. `rebind.attacker.example`), and the browser sends
`Host = <that attacker domain>`, which the fix correctly rejects. The attacker
does **not** control resolution of the bare label `testserver` in the victim's
resolver. Therefore `testserver` is **not** a confirmed *standard*
DNS-rebinding-deliverable bypass.

It **becomes** a real bypass under either of two non-default conditions:
1. The victim's environment resolves `testserver` to loopback (e.g. a corporate
   DNS alias, `/etc/hosts` entry, mDNS, or a DNS search-domain that appends a
   zone mapping `testserver` → `127.0.0.1`). Then an attacker webpage's
   `fetch('http://testserver:18899/...')` produces a loopback peer with
   `Host: testserver` → bypass. (Cross-origin reads remain CORS-limited, but
   blind/simple writes and same-origin tricks may still apply.)
2. Combined with the operator having set `VIBE_TRADING_ENABLE_SHELL_TOOLS=1`
   (the exact scenario PR #243's opt-in was designed for): the `testserver`
   Host bypass + opted-in shell tools restores the **full RCE chain** on the
   fixed v0.1.10. Demonstrated at runtime (Phase 3 of the variant script):
   `Host: testserver` → 201 → `POST /sessions/{id}/messages` → `BashTool`
   `subprocess.run(shell=True)` → `RCE_CONFIRMED` proof file written.

### Gap 2 (NOT a bypass, ruled out): direct cross-origin fetch to `localhost`

A `Host: localhost` request from a loopback peer is allowlisted **by design**
(legitimate dev access). An attacker page on `evil.com` doing
`fetch('http://localhost:18899/...')` would send `Host: localhost` and be
auth-bypassed, but this is a **cross-origin** request, so CORS governs it.
`_DEFAULT_CORS_ORIGINS` is loopback-only (`localhost:3000/5173/8000`,
`127.0.0.1:3000/5173/8000`) and `_parse_cors_origins` rejects `*` with
credentials enabled. JSON `POST /sessions` triggers a CORS preflight that
fails for `evil.com`, and even blind simple requests cannot read the
`session_id` needed to drive the RCE message. This is the reason the original
CVE used DNS rebinding (same-origin to defeat CORS), and the fix's Host check
closes exactly that. Not a bypass.

### Gap 3 (secondary, conditional): file tools are not gated by the shell-tools opt-in

`_SHELL_TOOL_NAMES = {"bash", "background_run"}`. The agent's `write_file`,
`edit_file`, `read_file`, and `skill_writer` tools are **not** gated by
`include_shell_tools`. If an attacker obtains an auth-bypassed session (e.g. via
the `testserver` gap), the agent could be driven to use `write_file` to
overwrite `agent/.env` (redirecting provider credentials) — bypassing the
`require_settings_write_auth` gate on the settings PUT endpoints, since the
agent's file tool writes the file directly rather than via the API. This is a
**secondary** issue: it only matters once an auth bypass exists, and on the
fixed version the default config still blocks the bash RCE primitive. Noted for
fix completeness, not claimed as an independent bypass.

### Gap 4 (ruled out): unauthenticated non-sensitive routes

Routes without `Depends(require_auth)`: `/health`, `/api`, `/skills`,
`/correlation`. These are informational/read-only (or trigger only known data
loaders) and do not reach the auth-bypass or RCE sink. `/system/shutdown` uses
`_require_shutdown_authorization` internally (bearer-or-loopback + cross-site
browser rejection). Not a variant of the CVE.

### Gap 5 (ruled out): WebSocket / event-stream surface

No `@app.websocket` endpoints exist in `api_server.py`. SSE event streams
(`/swarm/runs/{id}/events`) use `require_event_stream_auth` →
`_validate_api_auth`, which returns early for loopback — but only after the
Host middleware has already gated the loopback peer. So no middleware-bypassing
transport exists.

## Behavior before vs after the fix

| Request (loopback peer, no bearer) | v0.1.9 | v0.1.10 (fix) |
|---|---|---|
| `Host: rebind.attacker.test:port` `POST /sessions` | **201** (auth bypass) → RCE | **403** "Untrusted local API host" ✅ |
| `Host: testserver:port` `POST /sessions` | **201** (no Host check) | **201** (allowlist gap) ⚠️ |
| `Host: localhost:port` `POST /sessions` | 201 | 201 (by design) |
| `Host: testserver:port` + `VIBE_TRADING_ENABLE_SHELL_TOOLS=1` → message → bash | RCE | **RCE** (conditional) ⚠️ |
| `Host: testserver:port` (default config) → message → bash | RCE | no RCE (bash gated by opt-in) ✅ |
| `PUT /settings/llm` (auth-bypassed) | 422 (auth bypassed) | 401/403 (PR #245) ✅ |

## Threat-model scope (from `SECURITY.md`)

The repo's `SECURITY.md` defines scope as "the HKUDS/Vibe-Trading repository"
and asks for private responsible disclosure; it does **not** exclude any attack
class (no statement that loopbound/local attacks are out of scope). DNS
rebinding and loopback-trust bypasses are therefore in-scope. The `testserver`
gap is a legitimate finding within the project's stated scope.

## Completeness assessment

The fix **is effective against the standard DNS-rebinding vector** it was
written to stop (attacker-owned domain Host → 403), and the shell-tools opt-in
independently blocks the RCE primitive in the default configuration. However,
the fix is **not complete**: the `testserver` allowlist entry is an
over-broad trust grant that (a) breaks the fix's own invariant ("only
loopback-named Hosts are trusted") and (b) restores the full RCE chain on the
fixed version under the realistic opted-in-shell-tools configuration, whenever
`testserver` resolves to loopback in the victim's environment. Removing
`testserver` from the production allowlist (moving it to a test-only override)
closes the gap without affecting production behavior.