Bug 15530 – XMLHttpRequest should not support certain methods

• Status: RESOLVED
• Resolution: FIXED
• Component: XML

[Alexey Proskuryakov]

These methods make various XSS attacks possible.

   TRACE   http://www.kb.cert.org/vuls/id/867593
   TRACK   http://www.kb.cert.org/vuls/id/288308
   CONNECT http://www.kb.cert.org/vuls/id/150227

AFAIK the network layer blocks them on the Mac anyway, but I think we should explicitly check for these in XMLHttpRequest implementation itself.

[Julien Chaffraix]

Created attachment 17429 [details]
Patch + test case

Other browsers' behaviour :

- IE7 does not support these methods so raise an exception (message: invalid argument)

- Firefox raises a NS_ERROR_INVALID_ARG exception for track and trace (no exception for connect)

- Opera does not raise any exception


The patch makes us raise a SECURITY_ERR for the 3 methods (IE's behaviour and close to Firefox's).

[Alexey Proskuryakov]

Comment on attachment 17429 [details]
Patch + test case

+    layoutTestController.waitUntilDone();

This is not needed.

+    try {
+        xhr.open(method, "resources/1251.html", true);
+        xhr.send(null);

This test doesn't differentiate between open() and send() raising an exception, I think it should.

It may be slightly better to use sync XHR in the test, to maqke it more explicit that we don't wait for anything to finish.

[Julien Chaffraix]

Created attachment 17430 [details]
Patch updated with Ap's comments

> (From update of attachment 17429 [details] [edit])
> +    layoutTestController.waitUntilDone();

> This is not needed.

Removed.

> +    try {
> +        xhr.open(method, "resources/1251.html", true);
> +        xhr.send(null);

> This test doesn't differentiate between open() and send() raising an
> exception,
> I think it should.

In fact, the exception should be raised only for open() so I removed send() as it was not relevant.

> It may be slightly better to use sync XHR in the test, to maqke it more
> explicit that we don't wait for anything to finish.

The new test case uses a sync XHR.

[Alexey Proskuryakov]

Comment on attachment 17430 [details]
Patch updated with Ap's comments

r=me

[Alexey Proskuryakov]

Committed revision 27970.