Bugzilla – Full Text Bug Listing
| Summary: | Cross Site Request Forgery vulnerability (aka CSRF/XSRF) | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [RT] RAP | Reporter: | Sebastien Arod <sebastien.arod> | ||||||||
| Component: | RWT | Assignee: | Project Inbox <rap-inbox> | ||||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||||
| Severity: | normal | ||||||||||
| Priority: | P2 | CC: | ivan, lukasz.koniecki, mknauer, rsternberg, tbuschto | ||||||||
| Version: | 2.1 | ||||||||||
| Target Milestone: | 3.0 M6 | ||||||||||
| Hardware: | PC | ||||||||||
| OS: | Linux | ||||||||||
| Whiteboard: | sr211 | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Sebastien Arod
The connection id (cid) is NOT a security token. It's only purpose is to distinguish between connections within the same HttpSession. A cid is only valid within a single HttpSession. The cid alone does NOT provide access to the UISession. In order to hijack a UISession, an attacker would need the jsessionid AND the cid. Therefore I don't see that the cid could introduce a security issue. If you disagree, could you outline how a potential attack could work? Created attachment 233837 [details]
file:///home/sarod/Desktop/csrfdemo/csrfdemo.zip
csrfdemo
Hi Ralf, My first description was obviously not clear. Let's try to do better... RWT/RAP is currently vulnerable to CSRF (see https://www.owasp.org/index.php/CSRF for a description of CSRF and prevention methods). You can find in attachment a CSRF attack demo using the technique described here: http://blog.opensecurityresearch.com/2012/02/json-csrf-with-parameter-padding.html. The attachment is a zip file containing a README plus eclipse demo projects. To prevent CSRF the general recommendation from OWASP is to use a Synchronizer Token Pattern (https://www.owasp.org/index.php/Cross-Site_Request_Forgery_%28CSRF%29_Prevention_Cheat_Sheet#General_Recommendation:_Synchronizer_Token_Pattern). Ideally RAP should implement this pattern to prevent CSRF. The connection id is not the source of the vulnerability and the problem existed in earlier version of RAP. What I was trying to say in my initial comment is that connection ID could probably be modified to become a valid CSRF token. To do that connection ID should be: * Generated on server when server receives the initialization payload "rwt_initialize":true instead of being generated by client * A sufficiently long random token. OWASP suggest 256-bit BASE64 encoded hashes. * Propagated in payload rather than url. (See Disclosure of Token in URL parahraph on OWASP) Regards, -Seb Created attachment 233840 [details]
New CSRF Demo
Also RAP should only accept json requests when request header is "Content-Type:application/json" that would limit the risks of attacks using html forms. I was unable to reproduce the attack with the demo. Firefox, Chrome and IE all ask me to log in again. To explain how the demo is supposed to work: - User starts a RAP application. He is asked to log in for the HTTP session. - The attacker somehow gets the user to open given website in the same browser as the RAP application. - The website site sends two POST requests to the RAP server (it has to know the URL), one that creates a new UISession (using the same http-session and a random CID) and one that "clicks" on the button that was supposedly created in the response of the first request. As far as I can tell this would only work if the website is loaded from the same domain as the RAP application, or am I missing something? If you were able to get the user to enter an URL in the same TAB as the running application (i.e. a javascript: URL), you can prbably do pretty much do anything you want. Hi Tim, If I understand correctly you were able to reproduce the attack right? If not let me know if you need more details. Using the form.submit() on a form with the attacked application as the action url of the form the browser sends a post request as if clicked by the user. So the browser resend the cookie related to the attacked application domain and if the session cookie didn't expire yet the user http session is reused and user is authenticated. The html for the attack and the attacked application does NOT need to be hosted on the the same domain because the attack uses the javascript form.submit() method that is not restricted by the same origin policy (See http://en.wikipedia.org/wiki/Same_origin_policy#Corner_cases_and_exceptions). Thanks for pointing out this issue, Sebastien! After some adjustments I was able to reproduce the example. Even if the malicious website is hosted on a different domain, the requests issued from the iframes get the same jsessionid. So it's true that RAP applications are vulnerable to this kind of attacks. I currently think, however, that this vulnerability is not specific to RAP. It could happen with any other Servlet application as well, couldn't it? As for prevention of those attacks, would a server-generated cid really help? What if the client uses a script to process the responses and send the cid back? Couldn't a script create hidden forms at runtime and submit them? Hi Ralph, You are right non-ajax servlet applications are also vulnerable to CSRF. The recommendation (according to OWASP) for classic applications is to include a CSRF token in all sensitive forms submission. More generally there is 2 ways to trigger a POST request from client using javascript: 1/ Submit a using form.submit() method: This is not restricted by same origin policy. Attacker has to use tricks to compose the post payload that he wants from form hidden inputs. Attacker cannot do anything with the response (it's a fire and forget mechanism). 2/ Use the XmlHttpRequest API: Attacker has full control on: payload, content type. However this method is useless for attackers because it cannot be used cross domain (see Same origin policy: http://en.wikipedia.org/wiki/Same_origin_policy). So using a securely random server-generated cid would help because the attacker would only be able to send the first post request that would initialize the connection id but would not have the capability to read the cid from response and so additional requests would fail because the server would reject them saying that the cid is not valid. Thanks for your explanation, Sebastien. Actually, we initially planned to generate the cid on the server, but then decided for the current implementation the sake of backward compatibility and, most of all, to keep load testing simple. A load testing tool like JMeter would have to be configured to parse the JSON, store the cid and pass it along with every request. That's inconvenient but certainly possible, so I think we should change the implementation as suggested. I also agree with comment 5. Created attachment 233975 [details]
Patch for backporting to 2.1 maintenance branch
Add content type check before processing the POST request with commit aea2596e2c0ad9f99f956e21d3c1697f86abd014. Please review for backporting to 2.1 maintenance branch.
It seems that a content-type of application/json cannot be faked using an HTML form. I'm not sure about the exact reasons for that, but I did not succeed to get the attack working by setting the "enctype" attribute of the forms to "application/json" and also read at several places that this would not work [1,2]. So apparently, the patch prevents this kind of attacks. I consider it low-risk, since the web client and the Tabris clients already send the correct header. For requests that send non-JSON in a POST request (such as container auth redirects), this patch replaces the HTTP 500 "ParseException at 1:0" with an HTTP 400 "invalid content type", which is more appropriate anyway. For these reasons, +1 to backport this patch to 2.1.1. [1] http://portswigger.net/burp/help/suite_functions_csrfpoc.html [2] http://blog.opensecurityresearch.com/2012/02/json-csrf-with-parameter-padding.html (In reply to comment #13) > For these reasons, +1 to backport this patch to 2.1.1. Backported to 2.1-maintenance branch with commit 5f65c9d914306315349b49a4134ba2eba25fd445. I think we should make the suggested protocol change (generate cid on the server) for RAP 2.2, but since this is a significant change and the content-type patch seems to prevent the attack effectively, I wouldn't backport the cid change to 2.1.1. @Sebastien, do you think that's acceptable? (In reply to comment #15) > I think we should make the suggested protocol change (generate cid on the > server) for RAP 2.2, but since this is a significant change and the > content-type patch seems to prevent the attack effectively, I wouldn't > backport the cid change to 2.1.1. > > @Sebastien, do you think that's acceptable? Hi Ralph, From what I know it's not possible to do CSRF attack using autosubmit form post if Content-Type is checked to be application/json so I also think it's fine to do only the content type check in 2.1.1. However my expertise on CSRF is limited and I can be wrong. Also I would still do the cid change in 2.2 to make sure to comply with OWASP recommendations. While there are still some optimizations possible, the main issue of the bug has been addressed and this bug has been mentioned in the 2.2 release review [1]. Removing the "committer-only" flag now. [1] https://projects.eclipse.org/projects/rt.rap/releases/2.2.0/review RAP should not relay on the Content-Type header checking only. See: https://security.stackexchange.com/questions/10227/csrf-with-json-post/10231#10231. As mentioned in Comment 3 OWASP recommendation is to use a Synchronizer Token Pattern (https://www.owasp.org/index.php/Cross-Site_Request_Forgery_%28CSRF%29_Prevention_Cheat_Sheet#General_Recommendation:_Synchronizer_Token_Pattern). (In reply to Lukasz Koniecki from comment #18) > RAP should not relay on the Content-Type header checking only. See: > https://security.stackexchange.com/questions/10227/csrf-with-json-post/ > 10231#10231. > > As mentioned in Comment 3 OWASP recommendation is to use a Synchronizer > Token Pattern > (https://www.owasp.org/index.php/Cross- > Site_Request_Forgery_%28CSRF%29_Prevention_Cheat_Sheet#General_Recommendation > :_Synchronizer_Token_Pattern). That's why the bug is still open. The connection id (cid) is now generated on the server (change https://git.eclipse.org/r/#/c/44081/) and it's used as synchronization token as suggested in the recommendations. |