GET // brings the entire web server down with adapter-node

[lovasoa]

Describe the bug

There is a very serious denial of service security vulnerability in the latest adapter-node, where a simple request with the path set to // brings the entire web server down.

Reproduction

git clone git@github.com:lovasoa/sanipasse.git
cd sanipasse
git checkout 5ea330f
npm start

then

curl http://localhost:3000//

Logs

node:internal/errors:463
    ErrorCaptureStackTrace(err);
    ^

RangeError [ERR_HTTP_INVALID_STATUS_CODE]: Invalid status code: ERR_HTTP_INVALID_STATUS_CODE
    at new NodeError (node:internal/errors:370:5)
    at ServerResponse.writeHead (node:_http_server:275:11)
    at ServerResponse.writeHead (file:///home/olojkine/dev/sanipasse/build/index.js:11815:26)
    at ServerResponse._implicitHeader (node:_http_server:266:8)
    at ServerResponse.end (file:///home/olojkine/dev/sanipasse/build/index.js:11971:14)
    at Polka.onError (file:///home/olojkine/dev/sanipasse/build/index.js:12180:9)
    at next (file:///home/olojkine/dev/sanipasse/build/index.js:12244:42) {
  code: 'ERR_HTTP_INVALID_STATUS_CODE'
}

System Info

System:
    OS: Linux 5.13 Fedora 34 (Workstation Edition) 34 (Workstation Edition)
    CPU: (8) x64 Intel(R) Core(TM) i7-10510U CPU @ 1.80GHz
    Memory: 4.04 GB / 15.28 GB
    Container: Yes
    Shell: 5.1.0 - /bin/bash
  Binaries:
    Node: 16.5.0 - /usr/bin/node
    Yarn: 1.22.10 - /usr/bin/yarn
    npm: 7.19.1 - /usr/bin/npm
  Browsers:
    Firefox: 92.0
  npmPackages:
    @sveltejs/adapter-node: next => 1.0.0-next.51 
    @sveltejs/adapter-static: next => 1.0.0-next.20 
    @sveltejs/kit: next => 1.0.0-next.173 
    svelte: ^3.38.3 => 3.43.0

Severity

blocking all usage of SvelteKit

Additional Information

No response

[lovasoa]

This is the second time in just a few months that I am affected by a major security vulnerability in SvelteKit, coming from polka. The first time was in #1488

polka does not seem to be actively maintained and seems to have only one regular contributor : https://github.com/lukeed/polka/commits/master

Could a migration to a more solid web framework be envisioned, or is sveltekit tightly coupled with polka ?

[mohe2015]

Also please add a SECURITY.md sveltejs/svelte#6430 as this should not be reported publicly.

[lovasoa]

The original error comes from this line:

https://github.com/sveltejs/kit/blob/master/packages/adapter-node/src/kit-middleware.js#L10

new URL(req.url) can throw, and the onError handler in polka seems to handle this error incorrectly, trying to set res.statusCode to an invalid value.

[lukeed]

@lovasoa Again, any old error you find off the street isn't a denial of service attack. I'd appreciate you not trying to drag Polka every semblance of an opportunity you think you get, there are better ways to go about this than trying to inflate severity & cast doubt/shade on other projects. If you looked a bit closer, Polka is actively maintained and developed under the next branch/release, which SK uses.

[lovasoa]

I didn't mean to upset anyone, but I don't think calling this a severe security vulnerability is an overstatement. Currently, if you deploy a sveltekit application in its default configuration, anyone on the internet can bring it down with a single HTTP request.

This is the second time I find an instance of this in a few months, without actively looking. I don't want to cast shade on your project, but I don't think using it in SvelteKit is a good idea, if one keeps discovering new critical security vulnerabilities every few months because of it. Node has multiple battle-tested http frameworks to choose from, and I would feel safer running SvelteKit apps in production if it used one of those.

[lukeed]

You keep referencing a "2nd security issue" that doesn't exist. Polka@next has had the global try/catch in place for nearly 3 years. It was SK's choice (now reverted) to use polka@0.5.2 which clearly labels throughout its documentation that users must handle their own errors.

This ticket is the first Polka-related issue & didn't actually report to Polka directly. Instead, much like last time, you preferred to make a fuss downstream and throw in your own comments/opinions.

I don't actually care what SK uses, but it happens to use Polka because the projects' goals & philosophies re: perf/bloat align. And, hate to break it to you, but just like Polka, SvelteKit is still under a next umbrella, which means it's in development. So you've already accepted the risk of running non-"stable" code in production – and all of this is "battle-testing".

[lukeed]

FWIW, polka@1.0.0-next.21 is now published, which will no longer use an error.code as the response status code. This was an old Express ecosystem practice that fell out of practice & I never removed it. Thanks for the indirect report.

But the linked PR should still go ahead for better/more granular error handling.

[lovasoa]

I understand that you like your project, and don't like to see it being criticized. As a fellow opensource maintainer, I do respect your engagement, and the energy you put in your project. But pragmatically speaking, had SvelteKit used a more popular framework, both of these critical vulnerabilities would have been avoided.

And I think it is important to make a fuss, as you say, about severe vulnerabilities. This attitude towards security (dodging responsibility for an insecure design last time, trying to minimize it this time) is part of the reason why I don't feel great about SvelteKit using polka.

[lukeed]

All feedback, including criticism, is fine & welcomed. I'm not here telling you Polka is perfect. If it were, it wouldn't be under @next status still. I'm only taking issue with your method of reporting (or lack thereof) issues & taking each one as an opportunity to attack another project and/or the maintainers' decision to have used one.

To be clear, I'd be saying the same things if this were koa/tinyhttp/fastify too. The team's general approach here is to identify and fix issues upstream. @benmccann does an amazing job of this – he, and many others, are actively involved with vite's development now, which is directly a result of there being a number of vite-derived errors & bugs. Vite is new & currently undergoing its refinement/battle-testing phase. Is vite perfect? No, but getting there. Should SvelteKit move off vite? An emphatic no, even with/despite some work it's caused.

The "making a fuss" is not the report itself. It's your commentary that goes with it.

[shellscape]

I landed here out of my frustration with using Polka along side Svelte Kit, and am commenting here due to that specific context, and in relation to prior discussion on this issue.

But pragmatically speaking, had SvelteKit used a more popular framework, both of these critical vulnerabilities would have been avoided.

Seconded

I'm only taking issue with your method of reporting (or lack thereof) issues & taking each one as an opportunity to attack another project and/or the maintainers' decision to have used one.

Conflict of interest here, and this is on reason why I feel it was a poor choice to use your package in the project - you're viewing this through a lens of attack (which seems to be a trend in other projects), whereas an objective observer is viewing this through the lens of valid criticism. If you're unable to look at criticism of the project (polka) in that vein, then it should either be transferred to an org for active maintenance where it can be treated objectively, being a rather core dependency of SvelteKit, or it should be removed as a dependency.

The team's general approach here is to identify and fix issues upstream.

What about the maintenance of polka itself? next hasn't had an update since this issue was addressed and master is 142 commits behind next. It seems polka would be better served by an org.

All in all, having polka as the sole blessed Node server for adapter-node is extremely limiting and seeing discussions around polka's shortcomings play out in such a defensive manner is extremely disheartening.

[lukeed]

The defensiveness here came from the fact that multiple issues were opened here about Polka instead of raising/bringing those issue reports to Polka directly. They’ve since been fixed & published in @next immediately after the fix. I’ve said frequently that I don’t care what’s used here & I had no solicitation in its choice. Rich chose it because it’s light & relies on native APIs (iirc) and it’s remained that way since. There’s no other thought going into it — at least not by me.

[benmccann]

It's not clear to me if there are still any issues people have hit using Polka with SvelteKit? As far as I know, all issues have been addressed. If you're still hitting some issue, and there are no issues that are currently open, please open a new issue

[organisciak]

I'd like to express my appreciation for fixing this. I've been stumbling into the issue for weeks and was out of my league for addressing it properly.