CORS: any localhost port is trusted, and the server's own IP origin is refused

[lgnap]

This starts with a correction of something I told you twice, in #43 and in the thread on my fork. I was wrong, and I would rather retract it in public than let it quietly stand.

What I claimed, and why it is wrong

I said uploadToLibrary was open to CSRF: it calls xhr.send(formData) with no custom header, so the request is multipart/form-data, a CORS-simple content type, so no preflight, so a page anywhere could fire it.

Every step of that is true except the conclusion. It assumes CORS is advisory — that the browser is told not to read the response while the request still reaches the handler. Vert.x's CorsHandler does not work that way. It rejects a disallowed origin server-side, on actual requests and not only preflights, with a 403 before any handler runs.

I verified this against your exact configuration rather than reasoning about it again — a bare Vert.x 3.9 server with CorsHandler.create("http://localhost:.*") and allowedMethods(GET, POST), nothing else:

Origin: (none)                        -> 200   reached the handler
Origin: http://localhost:8080         -> 200
Origin: http://localhost:31337        -> 200
Origin: http://127.0.0.1:8080         -> 403
Origin: https://evil.example          -> 403
Origin: http://localhost.evil.example -> 403
Origin: http://evil.example:8080      -> 403

A browser always sends Origin on a cross-origin POST, so an attacker page gets a 403. The CSRF I described is not exploitable. Please do not spend time on it on my account.

The requests with no Origin reaching the handler are not a hole either: that is curl and scripts, which are not browser-mediated and already have whatever access the user has.

What the same table does show

Two things, and I am genuinely unsure whether either is worth your time.

1. Any localhost port is trusted. http://localhost:31337 is allowed, because http://localhost:.* matches every port. So any web origin on the machine — another project's dev server, a local tool with an XSS, anything the user has open on a localhost origin — can drive STUdio's API: list the library, download packs, upload, delete, operate the device. Nothing stands in the way, since there is no authentication.

How much that matters depends on your threat model. The counter-argument is that anything running locally already has the user's files. The counter to that is that a web page on a localhost origin is not local code execution: it is reached by visiting a URL, and it inherits the ability to drive an unauthenticated API that the loopback binding exists to protect. On an application deliberately restricted to the loopback, "what else is on this machine" is the surface that is left.

2. http://127.0.0.1:8080 is refused — the address the server itself binds. This one is a plain bug rather than a judgement call. A user who opens the UI at http://127.0.0.1:8080 instead of http://localhost:8080 gets a browser that sends Origin: http://127.0.0.1:8080 on every POST, so every state-changing call — upload, convert, remove, and the device operations — comes back 403. Reads mostly work, since browsers omit Origin on same-origin GETs. The UI half-works, in a way that is hard to diagnose from the outside.

This predates #43 and is not caused by it.

What a fix would look like

Both come from the same place: the allowed origin is written as a pattern over localhost rather than derived from what the server actually serves. The origin is known — it is listenHost() and LISTEN_PORT. Allowing exactly http://localhost:<port> and http://127.0.0.1:<port>, and nothing else, closes the port wildcard and fixes the IP spelling in one change.

One consequence worth knowing before you decide. Tightening it breaks the yarn start event bus. Under the dev server the page is served from :3000; proxy forwards /api and /locales server-side so those stay same-origin, but the SockJS connection goes straight to :8080 and is genuinely cross-origin. It needs http://localhost:3000 allowed. Gating that on the env=dev flag you already have would keep the production pattern tight, at the cost of a branch in the CORS setup.

A smaller point for completeness: x-requested-with is already in allowedHeaders, but nothing sends or requires it. Requiring it on state-changing routes would force a preflight and give a second, independent barrier. I mention it because it is already half-present, not because I think it is needed once the origin is exact.

The actual question

Is this worth addressing, or is it fine as it is?

I can see the argument for leaving it. Item 1 needs an attacker who already has a foothold in the user's browser on a local origin. Item 2 has an obvious workaround — use localhost. Against that, item 2 is a real user-visible failure with a confusing shape, and the fix for both is small and testable.

If you want it done, I am happy to write it, with the dev-mode branch and a test over the origin table above. If you would rather not, close this — after the correction at the top I would rather ask than assume I have found something.

[antoinevalentinHA]

Retracting it in public was the right call, and it cost you nothing here — the correction is more useful than the original claim would have been.

The measurements you asked for. Mine were against the running application with a real file part, on a sandboxed library, so they carry the step yours deliberately stopped short of:

Origin: http://evil.example       -> 403   no file written
Origin: (none)                    -> 200   file written
Origin: http://localhost:8080     -> 200   file written
Origin: http://localhost:3000     -> 200   file written
Origin: http://localhost:1        -> 200   file written
Origin: http://127.0.0.1:8080     -> 403
Origin: https://localhost:443     -> 403
Origin: http://localhost.evil.com -> 403

So the write really happens, not just the dispatch. Our tables agree everywhere they overlap.

To your actual question: yes, both, and item 2 is what decides it. A user who opens the UI at 127.0.0.1:8080 gets reads that work and every state-changing call refused — on the very address the server now binds. That is a real failure with a confusing shape, and 127.0.0.1 is if anything the more natural thing to type since #41. Item 1 then comes free from the same change, and your framing of it is right: a page on a localhost origin is not local code execution, and on an application deliberately confined to the loopback, what else is on the machine is the surface that remains.

Deriving the origin from listenHost() and LISTEN_PORT rather than pattern-matching localhost is the right shape. Please do write it, with the origin table as a test.

One thing before you do. I was uneasy about the dev-mode branch: it makes a security-relevant relaxation conditional on a flag that so far only swaps in the mock story teller. So I checked whether the dev server could carry the SockJS connection instead, and it can. react-scripts 3.0.1 loads src/setupProxy.js — paths.js:90, required at webpackDevServer.config.js:96 — and ships http-proxy-middleware 0.19.2, so a /eventbus entry with ws: true is available. Measured with both servers running:

GET :3000/eventbus/info           -> 200, same payload as :8080
GET :3000/api/library/infos       -> 200, proxied
WS  :3000/eventbus/.../websocket  -> 101 Switching Protocols

The upgrade succeeds through the proxy, identical to the direct connection. And it does not merely move the problem: the page is served from :3000 and talks to :3000, so the browser sends no Origin at all and CORS never enters. REACT_APP_EVENTBUS_ORIGIN and the flagged branch both become unnecessary, and there is one CORS configuration in every environment.

I would rather have that than the dev-mode branch. Whether it belongs in the same PR is your call.

On x-requested-with: agreed, not needed once the origin is exact. Leave it.

No rush on any of this — I still owe you reviews on #47 and #48, and #48 needs a rebase on b1af634; it is conflicting and its checks have not run.