data-method whitelist to block adding CSRF #403
Conversation
If an attacker has the ability to create link tags and attributes they have the ability to intercept the CSRF token, even if CSP is enabled. By setting the href to another site and adding `data-method='post'` the CSRF token will automatically be added to the request and sent to an attackers site. It is important to note that this is only applicable to `handleMethod` (`data-method`) call as when done through `handleRemote` (`data-remote`) the CSP will block the request to the attackers domain. This fix adds a configurable whitelist of hrefs to verify the link against before adding the CSRF token. The default is to allow everything to avoid break existing clients.
|
Should we be checking for the raw href here, or might there be a way to check for the domain of the href? |
|
@RSO that's a good point, maybe we can make it user friendly whitelisting domains instead of hrefs, I'll try this out and update the diff. |
It can be difficult to setup a whitelist of hrefs. This change adds in hostname matching so it should be friendlier to configure.
|
👍 |
A potential attacker that can set the href or data-method attributes can also use attack strings to cause any html to be created which could allow bypassing of the whitelist.
|
We got external feedback from a security researcher, the way action, name and value attributes are being set can also be used for unsafe injection. I'll be updating the PR accordingly. |
|
@bwillis Just to clarify (and unless I'm misunderstanding), this potential exploit only works if the application developer is allowing their users to submit raw HTML links with arbitrary attributes and passing it directly through to other users without doing any sort of escaping or sanitization, right? As in, you'd have to be calling |
|
@JangoSteve You're right. The reason we spend time on actually fixing this was because of a recent XSS that was discovered in our markdown parsing (https://hackerone.com/reports/46072). Without the fix in this pull request every XSS in our markdown parser would be turned into a full-blown CSRF (through CSP bypass). Do you have any hesitations that makes you doubt landing this fix? |
|
@RSO Not at all. Just wanted to make sure I had a good understanding of the issue and sense of the severity. |
|
Cool! |
If an attacker has the ability to create link tags and attributes they have the ability to intercept the CSRF token, even if CSP is enabled. By setting the href to another site and adding
data-method='post'the CSRF token will automatically be added to the request and sent to an attackers site. It is important to note that this is only applicable tohandleMethod(data-method) call as when done throughhandleRemote(data-remote) the CSP will block the request to the attackers domain.This fix adds a configurable whitelist of hrefs to verify the link against before adding the CSRF token. The default is to allow everything to avoid break existing clients.