From c730244902b60359c7f5c12bd35a4ff08465fb24 Mon Sep 17 00:00:00 2001 From: seb Date: Sun, 18 Oct 2026 20:54:00 +0200 Subject: [PATCH] u --- TODO.md | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/TODO.md b/TODO.md index bcf568a..d26bc88 100644 --- a/TODO.md +++ b/TODO.md @@ -133,3 +133,70 @@ Conversion order (bottom-up by dependency): (queries/schema stay mssql-specific, so likely keep it) - [ ] Decide export for `SCHEMA_SCRIPTS` (currently re-exported from `index.js` from the moved schema file) + + + + + + + + + + + + + + +I'll check how auth is applied across the routes. + +Viewed index.js:1-305 +Searched for "app\.(post|put|patch|delete|get|use|all)\(" +Viewed auth.js:1-132 + +The middleware skips auth on several path prefixes. Next I'll check which of those can write. + +Searched for "\.(post|put|patch|delete)\(" +Searched for "'/(api/)?(public|mcp|oauth|\.well-known)" + +Next: the telemetry public route, the `/oauth/register` handler, and the v1 router. + +Viewed telemetry.js:1-118 +Viewed mcp.js:360-460 +Searched for "(router|app)\.(post|put|patch|delete|use|all)\(|auth|token" +Viewed index.js:1-27 +Searched for "registerClient|export (async )?function" +Viewed mcp.js:455-540 +Searched for "db\.\w+|verifyIdToken|allow|jwt\.sign" + +**No. With no JWT, an attacker can still write to the database.** The checks I did are static code review only, and I haven't run anything. + +`authMiddleware` ([auth.js](file:///home/seb/src/dashback/src/api/middleware/auth.js#L49-L68)) lets several path groups through without a token. Most of them can't write to the SQL database, but two can. + +### 1. Open OAuth registration: `POST /oauth/register` +- This route is public, and it calls `mcpDb.registerClient` ([mcp.js:368](file:///home/seb/src/dashback/src/api/routes/mcp.js#L368)) to insert a client row. +- Anyone can create unlimited rows with no auth and no rate limit. +- `client_name`, `redirect_uris`, `grant_types` and `scope` all come straight from the request body. + +### 2. Auth bypass: `POST /oauth/authorize` +- The middleware calls `next()` on this route even with no token ([auth.js:70-79](file:///home/seb/src/dashback/src/api/middleware/auth.js#L70-L79)). +- The handler ([mcp.js:455-503](file:///home/seb/src/dashback/src/api/routes/mcp.js#L455-L503)) takes an `email` from the body when there's no `req.user`. +- If that email is on the allowlist, it writes an auth code (`createAuthCode`) for that user and redirects with it. **No password, Google token or JWT is checked.** +- An attacker can chain this with the previous hole: + 1. Register a client with `/oauth/register`. + 2. Post to `/oauth/authorize` with the victim's allowlisted email and their own PKCE `code_challenge`. + 3. Exchange the code at `/oauth/token`, which I haven't read. + 4. Call `/mcp` as that user. +- That would be a full impersonation if the MCP tools can write. Please confirm this by testing against a dev instance. +- Also, `new URL(redirect_uri)` runs with no try/catch and no check that `redirect_uri` matches the registered URIs. A bad value throws, and a valid one is an open redirect that sends the code to the attacker. + +### Not writing to the SQL database +- `/api/public/telemetry/crash` ([telemetry.js](file:///home/seb/src/dashback/src/api/routes/telemetry.js#L87-L104)) is public and unauthenticated. It writes to a file on disk rather than SQL, but there's no size or rate limit beyond `bodyLimit`, so it can fill the disk. +- `/v1` and `/api/v1` skip the middleware, but the routes I saw are all `GET`. The ones with data use `posAuthMiddleware`, and `/client`, `/pimage` and `/cimage` have no auth. +- `/api/auth/login` verifies a Google ID token and checks the allowlist. If `IMPLICIT_CREATE=true`, any valid Google account gets added to the allowlist ([login.js:107](file:///home/seb/src/dashback/src/api/routes/auth/login.js#L107)). Make sure that's off in production. + +### Suggested fixes +- Make `/oauth/authorize` POST require `req.user`, and remove the `email` fallback. +- Validate `redirect_uri` against the registered URIs for the client. +- Rate-limit `/oauth/register`, or gate it. + +Want me to implement these? I haven't changed any code. \ No newline at end of file