Fix null pointer dereference in WebSocket::connect()
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 5 Oct 2015 23:28:20 +0000 (23:28 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 5 Oct 2015 23:28:20 +0000 (23:28 +0000)
https://bugs.webkit.org/show_bug.cgi?id=149311
<rdar://problem/22748858>

Patch by Jiewen Tan <jiewen_tan@apple.com> on 2015-10-05
Reviewed by Chris Dumez.

Source/WebCore:

This is a merge of Blink r187441,
https://codereview.chromium.org/785933005

Test: http/tests/websocket/construct-in-detached-frame.html

* Modules/websockets/WebSocket.cpp:
(WebCore::WebSocket::connect):
Call function implemented below instead of duplicating the code.
* page/ContentSecurityPolicy.cpp:
(WebCore::ContentSecurityPolicy::shouldBypassMainWorldContentSecurityPolicy):
* page/ContentSecurityPolicy.h:
Factor the logic to check shouldBypassMainWorldContentSecurityPolicy into
a function in this class. Check Frame pointers are not null before getting
shouldBypassMainWorldContentSecurityPolicy via those pointers.
* page/EventSource.cpp:
(WebCore::EventSource::create):
This got fixed as a bonus.
* xml/XMLHttpRequest.cpp:
(WebCore::XMLHttpRequest::open):
This got fixed as a bonus too.

LayoutTests:

* http/tests/websocket/construct-in-detached-frame-expected.txt: Added.
* http/tests/websocket/construct-in-detached-frame.html: Added.
* http/tests/websocket/resources/construct-in-detached-frame.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@190588 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/http/tests/websocket/construct-in-detached-frame-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/websocket/construct-in-detached-frame.html [new file with mode: 0644]
LayoutTests/http/tests/websocket/resources/construct-in-detached-frame.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/Modules/websockets/WebSocket.cpp
Source/WebCore/page/ContentSecurityPolicy.cpp
Source/WebCore/page/ContentSecurityPolicy.h
Source/WebCore/page/EventSource.cpp
Source/WebCore/xml/XMLHttpRequest.cpp

index 077387c..e5fb77d 100644 (file)
@@ -1,3 +1,15 @@
+2015-10-05  Jiewen Tan  <jiewen_tan@apple.com>
+
+        Fix null pointer dereference in WebSocket::connect()        
+        https://bugs.webkit.org/show_bug.cgi?id=149311
+        <rdar://problem/22748858>
+
+        Reviewed by Chris Dumez.
+
+        * http/tests/websocket/construct-in-detached-frame-expected.txt: Added.
+        * http/tests/websocket/construct-in-detached-frame.html: Added.
+        * http/tests/websocket/resources/construct-in-detached-frame.html: Added.
+
 2015-10-05  Alexey Proskuryakov  <ap@apple.com>
 
         Revert LayoutTests parts of r190579, which were incorrect.
diff --git a/LayoutTests/http/tests/websocket/construct-in-detached-frame-expected.txt b/LayoutTests/http/tests/websocket/construct-in-detached-frame-expected.txt
new file mode 100644 (file)
index 0000000..c5f1631
--- /dev/null
@@ -0,0 +1,9 @@
+Construct a WebSocket in a detached frame. The test passes if there is no crash.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/http/tests/websocket/construct-in-detached-frame.html b/LayoutTests/http/tests/websocket/construct-in-detached-frame.html
new file mode 100644 (file)
index 0000000..34a8008
--- /dev/null
@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<script src="../resources/js-test-pre.js"></script>
+<script>
+    description('Construct a WebSocket in a detached frame. The test passes if there is no crash.');
+
+    window.jsTestIsAsync = true;
+
+    function detachIframe()
+    {
+        var testIframe = document.getElementById('testIframe');
+        testIframe.parentNode.remove(testIframe);
+    }
+
+    function done()
+    {
+        finishJSTest();
+    }
+</script>
+<iframe src="resources/construct-in-detached-frame.html" id="testIframe"></iframe>
+<script src="../resources/js-test-post.js"></script>
\ No newline at end of file
diff --git a/LayoutTests/http/tests/websocket/resources/construct-in-detached-frame.html b/LayoutTests/http/tests/websocket/resources/construct-in-detached-frame.html
new file mode 100644 (file)
index 0000000..ccb2b0a
--- /dev/null
@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<script>
+var parentWindow = parent;
+var webSocketClass = WebSocket;
+
+parentWindow.detachIframe();
+try {
+    new webSocketClass('ws://127.0.0.1/');
+} catch (e) {
+    parentWindow.console.log(e.message);
+}
+parentWindow.done();
+</script>
index f891183..40ea586 100644 (file)
@@ -1,3 +1,32 @@
+2015-10-05  Jiewen Tan  <jiewen_tan@apple.com>
+
+        Fix null pointer dereference in WebSocket::connect()        
+        https://bugs.webkit.org/show_bug.cgi?id=149311
+        <rdar://problem/22748858>
+
+        Reviewed by Chris Dumez.
+
+        This is a merge of Blink r187441,
+        https://codereview.chromium.org/785933005
+
+        Test: http/tests/websocket/construct-in-detached-frame.html
+
+        * Modules/websockets/WebSocket.cpp:
+        (WebCore::WebSocket::connect):
+        Call function implemented below instead of duplicating the code.
+        * page/ContentSecurityPolicy.cpp:
+        (WebCore::ContentSecurityPolicy::shouldBypassMainWorldContentSecurityPolicy):
+        * page/ContentSecurityPolicy.h:
+        Factor the logic to check shouldBypassMainWorldContentSecurityPolicy into 
+        a function in this class. Check Frame pointers are not null before getting 
+        shouldBypassMainWorldContentSecurityPolicy via those pointers.
+        * page/EventSource.cpp:
+        (WebCore::EventSource::create):
+        This got fixed as a bonus.
+        * xml/XMLHttpRequest.cpp:
+        (WebCore::XMLHttpRequest::open):
+        This got fixed as a bonus too.
+
 2015-10-05  Beth Dakin  <bdakin@apple.com>
 
         WebCore::IOSurface should ask the IOSurface for the pixel format instead of 
index 4c79b60..6465840 100644 (file)
@@ -238,11 +238,7 @@ void WebSocket::connect(const String& url, const Vector<String>& protocols, Exce
     }
 
     // FIXME: Convert this to check the isolated world's Content Security Policy once webkit.org/b/104520 is solved.
-    bool shouldBypassMainWorldContentSecurityPolicy = false;
-    if (is<Document>(*scriptExecutionContext())) {
-        Document& document = downcast<Document>(*scriptExecutionContext());
-        shouldBypassMainWorldContentSecurityPolicy = document.frame()->script().shouldBypassMainWorldContentSecurityPolicy();
-    }
+    bool shouldBypassMainWorldContentSecurityPolicy = ContentSecurityPolicy::shouldBypassMainWorldContentSecurityPolicy(*scriptExecutionContext());
     if (!scriptExecutionContext()->contentSecurityPolicy()->allowConnectToSource(m_url, shouldBypassMainWorldContentSecurityPolicy)) {
         m_state = CLOSED;
 
index fab49e4..c46118d 100644 (file)
@@ -37,6 +37,7 @@
 #include "PingLoader.h"
 #include "RuntimeEnabledFeatures.h"
 #include "SchemeRegistry.h"
+#include "ScriptController.h"
 #include "ScriptState.h"
 #include "SecurityOrigin.h"
 #include "SecurityPolicyViolationEvent.h"
@@ -1780,4 +1781,14 @@ bool ContentSecurityPolicy::experimentalFeaturesEnabled() const
 #endif
 }
 
+bool ContentSecurityPolicy::shouldBypassMainWorldContentSecurityPolicy(ScriptExecutionContext& context)
+{
+    if (is<Document>(context)) {
+        auto& document = downcast<Document>(context);
+        return document.frame() && document.frame()->script().shouldBypassMainWorldContentSecurityPolicy();
+    }
+    
+    return false;
+}
+    
 }
index dc3c951..cedf1e2 100644 (file)
@@ -130,6 +130,7 @@ public:
     String evalDisabledErrorMessage() const;
 
     bool experimentalFeaturesEnabled() const;
+    static bool shouldBypassMainWorldContentSecurityPolicy(ScriptExecutionContext&);
 
 private:
     void logToConsole(const String& message, const String& contextURL = String(), const WTF::OrdinalNumber& contextLine = WTF::OrdinalNumber::beforeFirst(), JSC::ExecState* = nullptr) const;
index cbca4fa..9f90b01 100644 (file)
@@ -85,11 +85,7 @@ RefPtr<EventSource> EventSource::create(ScriptExecutionContext& context, const S
     }
 
     // FIXME: Convert this to check the isolated world's Content Security Policy once webkit.org/b/104520 is solved.
-    bool shouldBypassMainWorldContentSecurityPolicy = false;
-    if (is<Document>(context)) {
-        Document& document = downcast<Document>(context);
-        shouldBypassMainWorldContentSecurityPolicy = document.frame()->script().shouldBypassMainWorldContentSecurityPolicy();
-    }
+    bool shouldBypassMainWorldContentSecurityPolicy = ContentSecurityPolicy::shouldBypassMainWorldContentSecurityPolicy(context);
     if (!context.contentSecurityPolicy()->allowConnectToSource(fullURL, shouldBypassMainWorldContentSecurityPolicy)) {
         // FIXME: Should this be throwing an exception?
         ec = SECURITY_ERR;
index 37e3d0d..04b3046 100644 (file)
@@ -499,12 +499,7 @@ void XMLHttpRequest::open(const String& method, const URL& url, bool async, Exce
     }
 
     // FIXME: Convert this to check the isolated world's Content Security Policy once webkit.org/b/104520 is solved.
-    bool shouldBypassMainWorldContentSecurityPolicy = false;
-    if (is<Document>(*scriptExecutionContext())) {
-        Document& document = downcast<Document>(*scriptExecutionContext());
-        if (document.frame())
-            shouldBypassMainWorldContentSecurityPolicy = document.frame()->script().shouldBypassMainWorldContentSecurityPolicy();
-    }
+    bool shouldBypassMainWorldContentSecurityPolicy = ContentSecurityPolicy::shouldBypassMainWorldContentSecurityPolicy(*scriptExecutionContext());
     if (!scriptExecutionContext()->contentSecurityPolicy()->allowConnectToSource(url, shouldBypassMainWorldContentSecurityPolicy)) {
         // FIXME: Should this be throwing an exception?
         ec = SECURITY_ERR;