Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spec says no media type parameters, but this creates a potential vulnerable #1547

Open
Relequestual opened this issue Mar 1, 2021 · 1 comment

Comments

@Relequestual
Copy link

Media type parameters are not allowed by the spec, explicitly forbidden. This includes charset.
In an effort to comply with this, I was looking into how to disable the default charset being set when using express.js.

Turns out, you cannot:
expressjs/express#3490 (comment)

There isn't a way to disable because it was a security vulnerability reported against us (https://nodesecurity.io/advisories/8).

Vulnerable versions of express do not specify a charset field in the content-type header while displaying 400 level response messages. The lack of enforcing user's browser to set correct charset, could be leveraged by an attacker to perform a cross-site scripting attack, using non-standard encodings, like UTF-7.

Later in the GH issue, there's a link to a more detailed article, which I can't claim to fully understand, nor do I have the time to do a deep dive on it: https://portswigger.net/research/json-hijacking-for-the-modern-web

Express.js is one of the most well-used node libraries, and it's prohibitively hard to work around this, especially within the bounds of a self-built application framework.

While it might be the "correct and valid" approach to say no media type parameters are allowed, are they doing any harm if included? It seems more, the inverse is true here.

@dgeb
Copy link
Member

dgeb commented Mar 25, 2021

The JSON spec states that:

JSON text exchanged between systems that are not part of a closed ecosystem MUST be encoded using UTF-8

Since JSON:API is of course based on JSON, this applies to us as well.

I talked with @gabesullice about this issue today. We're both in favor of adding an explicit exception for the charset, stating that if this parameter is present, its value MUST be UTF-8. And if it's not present, then UTF-8 should be assumed.

Thanks for raising this. I think it will be useful to address this explicitly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants