Closed Bug 718319 (CVE-2012-0440) Opened 13 years ago Closed 13 years ago

[SECURITY] JSON-RPC permits to bypass token checks and can lead to CSRF (no victim's action required)

Categories

(Bugzilla :: WebService, defect)

Product:
Bugzilla
Component:
WebService
Version:
3.5.1
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Bugzilla 3.6

People

(Reporter: netfuzzerr, Assigned: LpSolit)

References

Details

(Keywords: reporter-external, Whiteboard: [Bugzilla 3.4.x and older not affected][infrasec:csrf][ws:critical])

Attachments

(3 files, 3 obsolete files)

patch for 4.2 + tip, v3
2.28 KB, patch
LpSolit
: review+
Details | Diff | Splinter Review
patch for 4.0, v2
2.38 KB, patch
LpSolit
: review+
Details | Diff | Splinter Review
patch for 3.6, v2
2.22 KB, patch
dkl
: review+
Details | Diff | Splinter Review
User Agent: Mozilla/5.0 (Windows NT 6.1) AppleWebKit/535.16 (KHTML, like Gecko) Chrome/18.0.1003.1 Safari/535.16 Steps to reproduce: Hello, No security token is used when a POST request is made in jsonrpc.cgi thus allowing an attacker to add as CC, to visit and bugs taken private, or restricted to certain users. To explore this it is necessary that a User with privileges to edit bugs, so the attacker creating a page specially crafted to add CC. Reproduce: Do a POST Request in https://bugzilla.mozilla.org/jsonrpc.cgi, whit this content: {"version":"1.1","method":"Bug.update","id":2,"params":{"ids":[629221],"cc":{"add":["youremail@hack.com"]}}} Cheers, Mario
I thought about that too a few days ago, but I wasn't sure how a testcase would look like. XML-RPC is not vulnerable, right?
Assignee: general → webservice
Component: Bugzilla-General → WebService
I suppose that to fix this problem, login cookies generated for JSON-RPC and normal web browsing should be different.
Yes, I believe so. (In reply to Frédéric Buclin from comment #1) > I thought about that too a few days ago, but I wasn't sure how a testcase > would look like. XML-RPC is not vulnerable, right?
Frédéric, seeing your comments and the reason of this don't works in landfill.mozilla.org. It's seems this is not a security bug in bugzilla. Could you please confirm this?Is this a valid security issue?
(In reply to Mario Gomes from comment #4) > Could you please confirm this? Is this a valid security issue? I have been unable to write a testcase which works, so far. I tried: <html> <head> <title>attack</title> </head> <body> <form name="evil" action="http://localhost/bugzilla/jsonrpc.cgi" method="POST" enctype="application/json"> <input type="hidden" name="version" value="1.1"> <input type="hidden" name="id" value="2"> <input type="hidden" name="method" value="Bug.add_comment"> <input type="hidden" name="params" value='[{"id":1004,"comment":"attack"}]'> </form> <script>document.evil.submit();</script> </body> </html> I also tried enctype="text/plain" or removing enctype, but none of my attempts worked. Maybe my testcase is wrong. Which testcase did you use to validate this issue?
This is the problem. I have try using the Cross-Origin Resource Sharing(CORS) http://hacks.mozilla.org/2009/07/cross-site-xmlhttprequest-with-cors/. but, i don't have success. I'm trying exploit this yet.
Frédéric, could you please try this: <body onload="document.body.children[0].submit();"> <form action='https://bugzilla.mozilla.org/jsonrpc.cgi?version=1.1&method=Bug.update&params=[{"ids":[718319],"cc":{"add":["netfuzzer@gmail.com"]}}]' method="POST"> </form> </body> Works?
I got: No data.<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN"> <html><head> <title>200 OK</title> </head><body> <h1>OK</h1> <p>You don't have permission to access /jsonrpc.cgi on this server.</p> </body></html>
Anyone able to reproduce my PoC also?
Full exploit attached and tested on Chrome 18,Firefox 9 and Safari 5 don't works in IE(can be also exploited, but use anouther function, for more informations see http://msdn.microsoft.com/en-us/library/cc288060(v=vs.85).aspx). Reproduce: 1. Logged in Mozilla go to https://landfill.bugzilla.org/bugzilla-tip/show_bug.cgi?id=17039 2. See the bug. 3. Now, open the exploit attached. 4. Back to https://landfill.bugzilla.org/bugzilla-tip/show_bug.cgi?id=17039. 5. See the changes.
Only fixing, the first step is logged in landfill.bugzilla.org.
(In reply to Mario Gomes from comment #11) > Anyone able to reproduce my PoC also? Yes, your PoC works fine. I managed to make a bug public remotely without any action from the user, besides waiting for him to view my HTML page. So I think we should go towards what I suggested in comment 2 to prevent this vulnerability. Bugzilla 3.4.x and older are not affected as JSON-RPC has been implemented in Bugzilla 3.5.1, see bug 432907. This is a critical issue, because with the power of e.g. Bug.update (which you could use to CC yourself to security bugs, or remove security restrictions), Group.update (which you could use to make yourself an admin by altering the regexp), and the coming Bugzilla.set_parameters (which lets your edit *all* parameters), you could easily take control of any Bugzilla installation.
Severity: normal → critical
Status: UNCONFIRMED → NEW
Depends on: bz-json
Ever confirmed: true
Flags: blocking4.2+
Flags: blocking4.0.4+
Flags: blocking3.6.8+
Summary: CSRF in bugzilla.mozilla.org allow add CC. → JSON-RPC permits to bypass token checks and can lead to CSRF (no victim's action required)
Whiteboard: [Bugzilla 3.4.x and older not affected]
Target Milestone: --- → Bugzilla 3.6
Version: 4.3 → 3.5.1
Summary: JSON-RPC permits to bypass token checks and can lead to CSRF (no victim's action required) → [SECURITY] JSON-RPC permits to bypass token checks and can lead to CSRF (no victim's action required)
As near as I can tell, the PoC is not actually a security bug. LpSolit's reproduction depends on being able to do cross-domain XMLHttpRequests, which the browser disallows. If you're on a domain where CORS would allow you to make the request, then your administrator has explicitly trusted that domain and has essentially intentionally opened up this hole. If you are in any other situation where you can do a cross-domain POST via an XMLHttpRequest, then there is an extremely serious security issue in your browser. There's no risk of normal form posts to our jsonrpc.cgi, because they won't encode the sent data properly, and jsonrpc.cgi will reject it. I tested this and vetted it fairly carefully when writing the original implementation here. If you're talking about exploiting this via an attachment on a Bugzilla on the same domain, our existing attachment_base functionality is the solution there. It opens all attachments on a separate domain. This prevents attachments on a Bugzilla from running scripts against that Bugzilla, which is our general solution for this entire class of problems.
No, the CORS allows me to send the request, prohibits only have access to the server response. exemple(openning this in http://127.0.0.1/exemple.html: x=new XMLHttpRequest(); x.open('GET','http://www.bugzilla.org'); x.send(''); document.write(x.responseText); the GET request will be sent, but the browser will not allow i get the value of "x.responseText". So, i can send the POST from 127.0.0.1 to www.bugzilla.mozilla.org, but i can't get the response of Mozilla. (In reply to Max Kanat-Alexander from comment #15) > As near as I can tell, the PoC is not actually a security bug. LpSolit's > reproduction depends on being able to do cross-domain XMLHttpRequests, which > the browser disallows. If you're on a domain where CORS would allow you to > make the request, then your administrator has explicitly trusted that domain > and has essentially intentionally opened up this hole. > > If you are in any other situation where you can do a cross-domain POST via > an XMLHttpRequest, then there is an extremely serious security issue in your > browser. > > There's no risk of normal form posts to our jsonrpc.cgi, because they > won't encode the sent data properly, and jsonrpc.cgi will reject it. I > tested this and vetted it fairly carefully when writing the original > implementation here. > > If you're talking about exploiting this via an attachment on a Bugzilla on > the same domain, our existing attachment_base functionality is the solution > there. It opens all attachments on a separate domain. This prevents > attachments on a Bugzilla from running scripts against that Bugzilla, which > is our general solution for this entire class of problems.
Right. Okay, I didn't realize that browsers would allow cross-domain requests with XMLHttpRequest to actually ever leave the browser. However, here's the correct info: https://developer.mozilla.org/En/HTTP_access_control#Simple_requests The solution here is that we need to validate the Content-Type header as being the proper json-rpc header. None of the allowed browser cross-domain Content-Types should be accepted by jsonrpc.cgi, ever.
(In reply to Max Kanat-Alexander from comment #17) > The solution here is that we need to validate the Content-Type header as > being the proper json-rpc header. None of the allowed browser cross-domain > Content-Types should be accepted by jsonrpc.cgi, ever. What's the expected header for JSON-RPC? Also, I see no reason to reject remote read-only methods. They would be useful e.g. to display information of bugs in the See Also field.
(In reply to Frédéric Buclin from comment #20) > What's the expected header for JSON-RPC? Ah, I found the answer: "application/json", which indeed should fix this vulnerability, per http://security.stackexchange.com/a/10231
Attached patch patch, v1 (obsolete) — — Splinter Review
Tested on trunk and 4.2. I preferred to blacklist known content types which do not trigger a preflighted request than to whitelist application/json only and prevent legitimate scripts from working as there are probably several other valid content types which can be used in JSON-RPC requests.
Assignee: webservice → LpSolit
Status: NEW → ASSIGNED
Attachment #589542 - Flags: review?(mkanat)
There's also a "application/json-rpc" content-type as well... Not sure where its use differs from "application/json".
Don't forget application/javascript when doing JSONP which is a legal way to do cross-domain requests. dkl
As I said in comment 22, I don't need to list which content types are legal. My patch blacklists those which aren't.
(In reply to Frédéric Buclin from comment #21) > (In reply to Frédéric Buclin from comment #20) > > What's the expected header for JSON-RPC? > > Ah, I found the answer: "application/json", which indeed should fix this > vulnerability, per http://security.stackexchange.com/a/10231 We still need to add CSRF token checks. Just having content-type header checks is not enough, as the above link says.
Whiteboard: [Bugzilla 3.4.x and older not affected] → [Bugzilla 3.4.x and older not affected][infrasec:csrf][ws:critical]
(In reply to Reed Loden [:reed] (very busy) from comment #26) > We still need to add CSRF token checks. Just having content-type header > checks is not enough, as the above link says. The link doesn't say this check is not enough. It says: "So even with application/json checking, you can get pretty close to XSRF, if not completely there. [...] So whilst you can probably get away with it for now, I absolutely wouldn't recommend going forward without a proper anti-XSRF token system.". We would have to investigate how much code change is required for token checks before implementing it on branches, and the implications it could have on 3rd party applications using the JSON-RPC API.
Alias: CVE-2012-0440
I suppose for all WRITE operations, we require a token and add a new call to get this token generated using IP/userid/secret/etc. Will make all creates/updates a two step process but will be more secure in the long run. READ operations of course would still work without token. dkl
Comment on attachment 589542 [details] [diff] [review] patch, v1 Review of attachment 589542 [details] [diff] [review]: ----------------------------------------------------------------- ::: Bugzilla/WebService/Constants.pm @@ +176,5 @@ > +# The default content type for JSON-RPC is application/json. > +use constant CONTENT_TYPE_BLACKLIST => qw( > + text/plain > + application/x-www-form-urlencoded > + multipart/form-data A blacklist is asking for future trouble when HTML5 adds new types. In addition, a case-sensitive match is fooled by text/Plain Text/plain TEXT/PLAIN ... rest of the 2^9 variants Firefox coerces the <form enctype=> to lower-case but does not do so for XMLHttpRequest headers. I don't know what other browsers do but I would not stake a critical security check on them consistently lower-casing.
Attached patch patch for branches, v1.1 (obsolete) — — Splinter Review
Ah yes, you are right about the case-sensitive match. I fixed that. About blacklisting vs whitelisting, I'm not sure a whitelist is better as new types could appear, as you said yourself. Is there any official list of legal content types for JSON-RPC?
Attachment #589542 - Attachment is obsolete: true
Attachment #589542 - Flags: review?(mkanat)
Attachment #589615 - Flags: review?(mkanat)
(In reply to David Lawrence [:dkl] from comment #24) > Don't forget application/javascript when doing JSONP which is a legal way to > do cross-domain requests. JSONP is only used with GET, so cannot edit data already.
Attached patch patch, v2 (obsolete) — — Splinter Review
OK, per RFC 4627, application/json is the single official MIME type, so I removed the blacklist in favor of an explicit check for application/json. I saw almost no references to application/json-rpc, so I'm excluding it.
Attachment #589615 - Attachment is obsolete: true
Attachment #589615 - Flags: review?(mkanat)
Attachment #589637 - Flags: review?(mkanat)
Comment on attachment 589637 [details] [diff] [review] patch, v2 Review of attachment 589637 [details] [diff] [review]: ----------------------------------------------------------------- r+ for trunk. For the branches, instead of limiting it to a specific content-type, we should blacklist all the legal XMLHttpRequest content types. That way we're less likely to break legitimate integrations that are using JSON-RPC on the branches with some unexpected content-type. ::: Bugzilla/WebService/Server/JSONRPC.pm @@ +351,5 @@ > > Bugzilla->input_params($params); > > + if ($self->request->method eq 'POST') { > + # CSRF is possible when the Content-Type header is not application/json Note "CSRF is possible via XMLHttpRequest" @@ +356,5 @@ > + # (for example: text/plain or application/x-www-form-urlencoded). > + # application/json is the single official MIME type, per RFC 4627. > + my $content_type = $self->cgi->content_type; > + # The charset can be appended to the content type, so we use a regexp. > + if ($content_type !~ m{^application/json(;.*)?$}i) { Just checking that it starts with application/json should be enough.
Attachment #589637 - Flags: review?(mkanat) → review+
(In reply to Frédéric Buclin from comment #32) > OK, per RFC 4627, application/json is the single official MIME type, so I > removed the blacklist in favor of an explicit check for application/json. I > saw almost no references to application/json-rpc, so I'm excluding it. Just to respond to this: There are three versions of JSON-RPC, 1.0, 1.1, and 2.0. They have their own specification that is not an RFC. application/json-rpc is the MIME type from one of those specifications.
Comment on attachment 589615 [details] [diff] [review] patch for branches, v1.1 This is the patch I initially suggested, which blacklists the 3 content types which could be used with XMLHttpRequest. But dveditz said this was less safe. Please review it for branches, but based on what I found on the web and per my discussion with dveditz, there should be no reason to accept another content type than application/json.
Attachment #589615 - Attachment description: patch, v1.1 → patch for branches, v1.1
Attachment #589615 - Attachment is obsolete: false
Attachment #589615 - Flags: review?(mkanat)
(In reply to Max Kanat-Alexander from comment #34) > There are three versions of JSON-RPC, 1.0, 1.1, and 2.0. They have their > own specification that is not an RFC. application/json-rpc is the MIME type > from one of those specifications. Version 1.1 states that the recommended Content-Type when using POST is application/json: http://json-rpc.org/wd/JSON-RPC-1-1-WD-20060807.html#RequestHeaders Version 2.0 says it follows RFC 4627, which uses application/json: http://jsonrpc.org/spec.html It looks like application/json-rpc was only used for the never published version 1.2. Version 1.0 doesn't seem to enforce the value of Content-Type, which is a bit annoying. So I suppose it makes sense to use a blacklist on older branches. (In reply to Max Kanat-Alexander from comment #33) > r+ for trunk. As we didn't release 4.2 yet, it makes sense to take this patch for 4.2 too. I see no reason not to take it.
Flags: approval?
Flags: approval4.2?
Mario, Thank you for submitting this bug. This bug has been evaluated and is eligible for the bounty. Chris Hofmann will get in touch with you to coordinate the bounty payment.
Great - Thank you very much! (In reply to Michael Coates [:mcoates] from comment #37) > Mario, > > Thank you for submitting this bug. This bug has been evaluated and is > eligible for the bounty. Chris Hofmann will get in touch with you to > coordinate the bounty payment. (In reply to Michael Coates [:mcoates] from comment #37) > Mario, > > Thank you for submitting this bug. This bug has been evaluated and is > eligible for the bounty. Chris Hofmann will get in touch with you to > coordinate the bounty payment.
Great - Thank you very much! (In reply to Michael Coates [:mcoates] from comment #37) > Mario, > > Thank you for submitting this bug. This bug has been evaluated and is > eligible for the bounty. Chris Hofmann will get in touch with you to > coordinate the bounty payment. (In reply to Michael Coates [:mcoates] from comment #37) > Mario, > > Thank you for submitting this bug. This bug has been evaluated and is > eligible for the bounty. Chris Hofmann will get in touch with you to > coordinate the bounty payment.
Comment on attachment 589615 [details] [diff] [review] patch for branches, v1.1 Review of attachment 589615 [details] [diff] [review]: ----------------------------------------------------------------- This fixes the bug as far as we have actually reproduced it, and it (hopefully) won't be breaking clients who are setting things properly. I'm fine with having a less-complete fix on the branches unless we have another PoC that can get past this patch. ::: Bugzilla/WebService/Server/JSONRPC.pm @@ +365,5 @@ > + # There are some other content types which must also be banned for > + # security reasons. > + my $content_type = $self->cgi->content_type; > + # The charset can be appended to the content type, so we use a regexp. > + if (grep { $content_type =~ m{$_}i } CONTENT_TYPE_BLACKLIST) { Just to be safe, it's probably best to do \Q \E around $_ there.
Attachment #589615 - Flags: review?(mkanat) → review+
(In reply to Frédéric Buclin from comment #36) > Version 2.0 says it follows RFC 4627, which uses application/json: > http://jsonrpc.org/spec.html A few things about that: (1) That's the spec for JSON-RPC itself, not for JSON-RPC Over HTTP, which is a separate standard that's still being worked on. (2) The 2.0 standard has evolved over time, and some clients may be sending as application/json-rpc, so we should accept it.
(In reply to Max Kanat-Alexander from comment #41) > (1) That's the spec for JSON-RPC itself, not for JSON-RPC Over HTTP, which > is a separate standard that's still being worked on. > > (2) The 2.0 standard has evolved over time, and some clients may be > sending as application/json-rpc, so we should accept it. Thanks for the clarifications. Does it mean we should accept application/json-rpc for trunk + 4.2 too? Per my discussion with dveditz, it seemed it was fine to decide what we wanted as valid Content-Type values, and what we didn't want.
Flags: approval4.0?
Flags: approval3.6?
Blocks: 720746
(In reply to Frédéric Buclin from comment #42) > Thanks for the clarifications. Does it mean we should accept > application/json-rpc for trunk + 4.2 too? Yeah, I think that would be a good idea.
I have no problem with adding application/json-rpc to the whitelist if you think that will help someone.
I fixed the comment to mention XMLHttpRequest, and I also included application/json-rpc as a legal Content-Type value. Carrying forward mkanat's r+.
Attachment #589637 - Attachment is obsolete: true
Attachment #592723 - Flags: review+
I added \Q \E around $_ as suggested by mkanat. Carrying forward mkanat's r+.
Attachment #589615 - Attachment is obsolete: true
Attachment #592726 - Flags: review+
On 3.6, the patch applied cleanly in JSONRPC.pm (with some fuzz), but when I checked, the code was applied at the wrong place. So here is a patch for 3.6 specifically. The reason for the fuzz is that in 3.6, JSONRPC.pm only accepts the POST method, and so the code is a bit different. It's a trivial backport of the patch for 4.0.
Attachment #592737 - Flags: review?(mkanat)
Attachment #592737 - Flags: review?(dkl)
(In reply to Frédéric Buclin from comment #47) > Created attachment 592737 [details] [diff] [review] > patch for 3.6, v2 > > On 3.6, the patch applied cleanly in JSONRPC.pm (with some fuzz), but when I > checked, the code was applied at the wrong place. So here is a patch for 3.6 > specifically. The reason for the fuzz is that in 3.6, JSONRPC.pm only > accepts the POST method, and so the code is a bit different. It's a trivial > backport of the patch for 4.0. The patch works for 3.6 as far as using text/plain gives the proper error message. When using the other blacklisted content types, jsonrpc.cgi dies and gives the following error in the log: [Mon Jan 30 18:29:55 2012] [error] [client 127.0.0.1] [Mon Jan 30 18:29:55 2012] jsonrpc.cgi: Malformed multipart POST: data truncated Should we worry about this as it still fixes the vulnerability either way but it would be nice to see a proper error message when json::rpc doesn't understand the content-type. The error is coming from CGI.pm fwiw. dkl
Comment on attachment 592737 [details] [diff] [review] patch for 3.6, v2 Review of attachment 592737 [details] [diff] [review]: ----------------------------------------------------------------- r=dkl as this accomplishes the task of blocking the CSRF attack. I am not sure it is worth worrying about the other content-types throwing the CGI.pm error as this would happen up until now and anyway wanting their JSONRPC calls to work would not do it that way. dkl
Attachment #592737 - Flags: review?(dkl) → review+
Comment on attachment 592737 [details] [diff] [review] patch for 3.6, v2 As the goal is to ban these "illegal" content types, I don't care too much if the user doesn't get the error message in this case.
Attachment #592737 - Flags: review?(mkanat)
Flags: approval?
Flags: approval4.2?
Flags: approval4.2+
Flags: approval4.0?
Flags: approval4.0+
Flags: approval3.6?
Flags: approval3.6+
Flags: approval+
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/ modified Bugzilla/WebService/Server/JSONRPC.pm modified template/en/default/global/user-error.html.tmpl Committed revision 8103. Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.2/ modified Bugzilla/WebService/Server/JSONRPC.pm modified template/en/default/global/user-error.html.tmpl Committed revision 8021. Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.0/ modified Bugzilla/WebService/Constants.pm modified Bugzilla/WebService/Server/JSONRPC.pm modified template/en/default/global/user-error.html.tmpl Committed revision 7687. Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/3.6/ modified Bugzilla/WebService/Constants.pm modified Bugzilla/WebService/Server/JSONRPC.pm modified template/en/default/global/user-error.html.tmpl Committed revision 7276.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Hey Frédéric, Only a doubt, the final release with the patch for this bug will be out in mid July/August,right?
(In reply to Mario Gomes from comment #53) > Only a doubt, the final release with the patch for this bug will be out in > mid July/August,right? We are releasing 4.2rc2 today, and 4.2 final should be released in the second half of February.
Security advisory sent, removing sec flag.
Group: bugzilla-security
Flags: sec-bounty+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: