OAuth2 login (clickhouse-client): loopback /start leaks CSRF state and OIDC discovery response is unbounded

[Selfeer]

OAuth2 login (clickhouse-client): loopback /start leaks CSRF state and OIDC discovery response is unbounded

Follow-up audit of PR #1606 after the first-round fixes. Two Medium defects remain.

---

Defect 1 — Medium: Loopback /start leaks full auth URL (including CSRF state) to any local process

Impact: on a shared host, a local unprivileged process can read the in-flight OAuth state via the loopback server and race a forged /callback before the legitimate browser redirect — account-confusion / login hijack during --login=browser.

Affected code: src/Client/OAuthFlowRunner.cpp — AuthCodeHandler::handleRequest, runOAuthAuthCodeFlow.

if (path == "/start")
{
    std::string target;
    {
        std::lock_guard<std::mutex> lock(state.mtx);
        target = state.auth_url;
    }
    response.redirect(target, Poco::Net::HTTPResponse::HTTP_FOUND);
}

std::string auth_url
    = creds.auth_uri
    + "?response_type=code"
      // ...
    + "&state=" + csrf_state;

Why it's a defect: callback acceptance relies on received_state == csrf_state, but /start redirects to state.auth_url, which embeds csrf_state. Any local process that discovers the loopback port can GET /start, read Location:, and deliver a matching /callback?code=...&state=... ahead of the legitimate redirect.

Steps to reproduce:

1. Start clickhouse-client --login=browser on a shared host.
2. Local process on the same host finds the loopback port and issues GET /start.
3. That process reads the auth URL + state from the Location: header.
4. It drives the OAuth flow (or races the real browser) and delivers /callback?code=<attacker-code>&state=<leaked-state> first.
5. Client accepts the callback (state matches) and binds to the attacker's identity.

Expected: /start must not disclose the auth URL or state to an unauthenticated local caller.

Actual: /start returns HTTP 302 with Location: containing the full auth URL and state=<csrf_state> to any caller on the loopback socket.

Fix direction: remove the server-side redirect indirection and launch the browser directly with the auth URL (URL never leaves the process). If /start must stay, gate it behind a one-time unguessable launch token required on both /start and /callback.

Regression test direction: loopback-handler test asserting GET /start does not return a state-bearing redirect, and that a callback crafted from information obtainable via /start cannot complete the flow.

---

Defect 2 — Medium: OIDC discovery response is unbounded (memory-exhaustion DoS)

Impact: a malicious or misconfigured discovery endpoint can force unbounded memory growth in clickhouse-client during --login=device with discovery enabled, up to OOM termination.

Affected code: src/Client/OAuthProviderPolicy.cpp — fetchDeviceEndpointFromIssuer.

auto & stream = session.receiveResponse(response);
Poco::StreamCopier::copyToString(stream, body);
// ...
auto result = parser.parse(body);

Why it's a defect: discovery uses unbounded Poco::StreamCopier::copyToString and parses afterward. No byte-size guard on this path, even though postOAuthForm in the same PR already implements a bounded-copy pattern.

Steps to reproduce:

1. Configure --oauth-credentials with no device_authorization_uri (forces discovery) and an issuer pointing at an attacker-controlled / misbehaving endpoint.
2. Run clickhouse-client --login=device.
3. Endpoint serves a very large (or chunked-forever) body at .well-known/openid-configuration.
4. Observe RSS of the client growing proportional to response size.

Expected: discovery fetch enforces a hard maximum body size and fails fast with AUTHENTICATION_FAILED on overflow, consistent with postOAuthForm.

Actual: full response body is buffered into std::string with no cap before parsing.

Fix direction: apply the same bounded-copy pattern used in postOAuthForm (hard max bytes + explicit AUTHENTICATION_FAILED on overflow). Consider centralizing the helper so all OAuth HTTP reads share the same limit.

Regression test direction: mock discovery endpoint returning a payload over the limit; assert bounded failure without unbounded RSS growth. Covers both generic OIDC and Google discovery paths.

---

References

• PR: Add OAuth2 login to clickhouse-client (--login / --login=device) #1606