XHR follows 307 redirect on DELETE cross-domain

RESOLVED INVALID

Status

()

defect
RESOLVED INVALID
8 years ago
8 years ago

People

(Reporter: julian.reschke, Assigned: briansmith)

Tracking

({APIchange, compat})

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:moderate])

Attachments

(1 attachment)

2.80 KB, text/javascript
Details
Reporter

Description

8 years ago
Posted file test case
It seems that a DELETE request redirected with status 307 causes the redirected request to be sent to the target, even if it's cross-domain.

See attached test case, tested with mozilla-central as of today.

Note that the fixes for 598304 and 676059 should fix the issue.
Reporter

Updated

8 years ago
Summary: XHR followes 307 redirect on DELETE cross-domain → XHR follows 307 redirect on DELETE cross-domain
Reporter

Comment 1

8 years ago
(In reply to Julian Reschke from comment #0)
> Note that the fixes for 598304 and 676059 should fix the issue.

(confirmed)
No longer blocks: 598304
Depends on: 598304, 676059
Whiteboard: [sg:high]
The testcase is all chrome code. I don't think that we'll follow the redirect if a webpage was initiating the XHR.

So I don't think this is a security problem at all.
I just talked to Jonas about this. I do think it is a security problem and we should investigate changing the XHR in chrome to match the web behavior regarding blocking cross-domain redirects, but we need to investigate the compatibility impact.

Updated

8 years ago
Assignee: nobody → julian.reschke

Comment 4

8 years ago
Julian - assigned this to you based on the fact that you seem to be fixing the bugs that this depends on.
Reporter

Updated

8 years ago
No longer depends on: 598304
Reporter

Updated

8 years ago
Assignee: julian.reschke → nobody
Reporter

Comment 5

8 years ago
(In reply to Josh Aas (Mozilla Corporation) from comment #4)
> Julian - assigned this to you based on the fact that you seem to be fixing
> the bugs that this depends on.

Josh; fixing 676059 will fix the problem as observed with the test case. 

That being said, I currently have no clue at which layer the cross-origin restrictions kick in, so somebody else should have a look at this.
Jonas wrote:
> I'm comfortable with changing our behavior for chrome such
> that we fire an event when the cross-origin redirect happens
> and let the chrome-code opt in to following that redirect.

> I wouldn't call this "match the content XHR" as we have no
> such event right now.
>
> I would not be comfortable simply blocking cross-origin
> redirects without giving the chrome-code a way to opt back
> in to it.
>
> Also, we need to be prepared for this breaking addons, in
> which case we might need to re-evaluate our strategy.

So, it seems like we need to do the following to resolve this bug:
1. Implement the event Jonas mentions with his specified semantics for overriding the default behavior in both chrome XHR and content XHR.
2. Change the default behavior to be to cancel the request if the request is cross-origin for both chrome XHR and content XHR, instead of just for content XHR.

I think this might be sg:moderate, not sg:high, if the dangerous behavior occurs only for addons. (I don't know if that is the case yet.)
Whiteboard: [sg:high] → [sg:high?]
This should only be affecting addons and chrome code, yeah. If it doesn't, please file a separate bug and cc me.
Reporter

Comment 8

8 years ago
(In reply to Jonas Sicking (:sicking) from comment #2)
> The testcase is all chrome code. I don't think that we'll follow the
> redirect if a webpage was initiating the XHR.
> 
> So I don't think this is a security problem at all.

I just run an ad-hoc test, and indeed when run from XHR, the redirect does not happen - so the code layer above blocks the request.

(Note: there is a prompt, and when the user cancels, the XHR request does not throw but returns with status 307).
Reporter

Comment 9

8 years ago
The problem as observed by the test is gone now that bug is fixed. I believe we could close this bug once we have a test case. For xpc-initiated requests, we could extend test_XHR_redirects.js (should I?). Dunno where to place other tests though.

Updated

8 years ago
Assignee: nobody → bsmith
Whiteboard: [sg:high?] → [sg:moderate]
Reporter

Updated

8 years ago
Depends on: 696849
Is there anything more to do now that bug 696849 has been fixed? That means the testcase is demonstrating that things are working correctly, right?
To expand on the last question: Content XHR has been confirmed by the tests in bug 696849 to be working correctly, right?

Has this bug now morphed into "make Chrome XHR work like content XHR in the way it follows redirects", or is that a different bug?

If so, we should change the summary and remove the security-group flag to make the bug public.
Reporter

Comment 12

8 years ago
(In reply to Brian Smith (:bsmith) from comment #11)
> To expand on the last question: Content XHR has been confirmed by the tests
> in bug 696849 to be working correctly, right?

I believe so.

> Has this bug now morphed into "make Chrome XHR work like content XHR in the
> way it follows redirects", or is that a different bug?

I have no idea.
 
> ...

Comment 13

8 years ago
I would prefer that we file a new bug rather than morph this one.
Resolved invalid because this bug was originally filed against content XHR, which enforces the restriction correctly. See bug 716133 about making chrome and content XHR work the same regarding cross-domain redirects.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → INVALID
Group: core-security
You need to log in before you can comment on or make changes to this bug.