676829 - XHR follows 307 redirect on DELETE cross-domain

Status: RESOLVED INVALID

(Core :: Networking: HTTP, defect)

Description

[Julian Reschke]

Attachment: 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.

Comment 1

[Julian Reschke]

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

(confirmed)

Comment 2

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

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.

Comment 3

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

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.

Comment 4

[Josh Aas]

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

Comment 5

[Julian Reschke]

(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.

Comment 6

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

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.)

Comment 7

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

This should only be affecting addons and chrome code, yeah. If it doesn't, please file a separate bug and cc me.

Comment 8

[Julian Reschke]

(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).

Comment 9

[Julian Reschke]

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.

Comment 10

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

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?

Comment 11

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

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.

Comment 12

[Julian Reschke]

(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

[Josh Aas]

I would prefer that we file a new bug rather than morph this one.

Comment 14

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

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.