Changeset 64871 in webkit

Timestamp:

Aug 6, 2010 2:52:03 PM (14 years ago)

Author:

andersca@apple.com

Message:

Detect invalid CoreIPC messages and call didReceiveInvalidMessage
​https://bugs.webkit.org/show_bug.cgi?id=43643
<rdar://problem/7891069>

Reviewed by Adam Roben.

• Platform/CoreIPC/ArgumentDecoder.cpp:

(CoreIPC::ArgumentDecoder::alignBufferPosition):
If we can't correctly align the buffer position, mark the decoder as invalid.

• Platform/CoreIPC/ArgumentDecoder.h:

(CoreIPC::ArgumentDecoder::isInvalid):
Check if the argument decoder is valid.

(CoreIPC::ArgumentDecoder::markInvalid):
Mark the argument decoder as invalid, by setting its buffer position past its end position.

• Platform/CoreIPC/Connection.cpp:

(CoreIPC::Connection::dispatchMessages):
Check if m_client is null before dispatching messages. If an argument decoder was marked invalid, call
Connection::Client::didReceiveInvalidMessage.

• Platform/CoreIPC/Connection.h:

(CoreIPC::Connection::Message::releaseArguments):
Rename destroy to releaseArguments and make it return a PassOwnPtr.

• UIProcess/WebProcessProxy.cpp:

(WebKit::WebProcessProxy::~WebProcessProxy):
Call releaseArguments instead of destroy.

(WebKit::WebProcessProxy::didReceiveInvalidMessage):
Kill the web process and invalidate its connection.

• WebProcess/WebProcess.cpp:

(WebKit::WebProcess::didReceiveInvalidMessage):
Don't do anything, if the UI process is sending invalid messages there's not much we can do.

• WebProcess/WebProcess.h:

Location:

trunk/WebKit2

Files:

• ChangeLog (modified) (1 diff)
• Platform/CoreIPC/ArgumentDecoder.cpp (modified) (1 diff)
• Platform/CoreIPC/ArgumentDecoder.h (modified) (1 diff)
• Platform/CoreIPC/Connection.cpp (modified) (2 diffs)
• Platform/CoreIPC/Connection.h (modified) (2 diffs)
• UIProcess/WebProcessProxy.cpp (modified) (2 diffs)
• UIProcess/WebProcessProxy.h (modified) (1 diff)
• WebProcess/WebProcess.cpp (modified) (1 diff)
• WebProcess/WebProcess.h (modified) (1 diff)

trunk/WebKit2/ChangeLog

@@ +1 @@
+2010-08-06  Anders Carlsson  <andersca@apple.com>
+
+        Reviewed by Adam Roben.
+
+        Detect invalid CoreIPC messages and call didReceiveInvalidMessage
+        https://bugs.webkit.org/show_bug.cgi?id=43643
+        <rdar://problem/7891069>
+
+        * Platform/CoreIPC/ArgumentDecoder.cpp:
+        (CoreIPC::ArgumentDecoder::alignBufferPosition):
+        If we can't correctly align the buffer position, mark the decoder as invalid.
+
+        * Platform/CoreIPC/ArgumentDecoder.h:
+        (CoreIPC::ArgumentDecoder::isInvalid):
+        Check if the argument decoder is valid.
+
+        (CoreIPC::ArgumentDecoder::markInvalid):
+        Mark the argument decoder as invalid, by setting its buffer position past its end position.
+
+        * Platform/CoreIPC/Connection.cpp:
+        (CoreIPC::Connection::dispatchMessages):
+        Check if m_client is null before dispatching messages. If an argument decoder was marked invalid, call
+        Connection::Client::didReceiveInvalidMessage.
+
+        * Platform/CoreIPC/Connection.h:
+        (CoreIPC::Connection::Message::releaseArguments):
+        Rename destroy to releaseArguments and make it return a PassOwnPtr.
+
+        * UIProcess/WebProcessProxy.cpp:
+        (WebKit::WebProcessProxy::~WebProcessProxy):
+        Call releaseArguments instead of destroy.
+
+        (WebKit::WebProcessProxy::didReceiveInvalidMessage):
+        Kill the web process and invalidate its connection.
+
+        * WebProcess/WebProcess.cpp:
+        (WebKit::WebProcess::didReceiveInvalidMessage):
+        Don't do anything, if the UI process is sending invalid messages there's not much we can do.
+
+        * WebProcess/WebProcess.h:
+
 2010-08-06  Anders Carlsson  <andersca@apple.com>
 

trunk/WebKit2/Platform/CoreIPC/ArgumentDecoder.cpp

@@ -70 +70 @@
     if (buffer + size > m_bufferEnd) {
         // We've walked off the end of this buffer.
-        m_bufferPos = m_bufferEnd;
+        markInvalid();
         return false;
     }

trunk/WebKit2/Platform/CoreIPC/ArgumentDecoder.h

@@ -41 +41 @@
 
     uint64_t destinationID() const { return m_destinationID; }
+
+    bool isInvalid() const { return m_bufferPos > m_bufferEnd; }
+    void markInvalid() { m_bufferPos = m_bufferEnd + 1; }
 
     bool decodeBytes(Vector<uint8_t>&);

trunk/WebKit2/Platform/CoreIPC/Connection.cpp

@@ -238 +238 @@
     // Dispatch messages.
     for (size_t i = 0; i < incomingMessages.size(); ++i) {
+        // If someone calls invalidate while we're invalidating messages, we should stop.
+        if (!m_client)
+            return;
+        
         IncomingMessage& message = incomingMessages[i];
-        ArgumentDecoder* arguments = message.arguments();
+        OwnPtr<ArgumentDecoder> arguments = message.releaseArguments();
 
         if (message.messageID().isSync()) {
@@ -254 +258 @@
             
             // Hand off both the decoder and encoder to the client..
-            m_client->didReceiveSyncMessage(this, message.messageID(), arguments, replyEncoder.get());
+            m_client->didReceiveSyncMessage(this, message.messageID(), arguments.get(), replyEncoder.get());
             
+            // FIXME: If the message was invalid, we should send back a SyncMessageError.
+            ASSERT(!arguments->isInvalid());
+
             // Send the reply.
             sendMessage(MessageID(CoreIPCMessage::SyncMessageReply), replyEncoder.release());
         } else
-            m_client->didReceiveMessage(this, message.messageID(), arguments);
-
-        message.destroy();
+            m_client->didReceiveMessage(this, message.messageID(), arguments.get());
+
+        if (arguments->isInvalid())
+            m_client->didReceiveInvalidMessage(this, message.messageID());
     }
 }

trunk/WebKit2/Platform/CoreIPC/Connection.h

@@ -69 +69 @@
         virtual ~Client() { }
 
-    public:        
+    public:
         virtual void didClose(Connection*) = 0;
+        virtual void didReceiveInvalidMessage(Connection*, MessageID) = 0;
     };
 
@@ -115 +116 @@
         T* arguments() const { return m_arguments; }
         
-        void destroy() 
+        PassOwnPtr<T> releaseArguments()
         {
-            delete m_arguments;
+            T* arguments = m_arguments;
+            m_arguments = 0;
+
+            return arguments;
         }
         

trunk/WebKit2/UIProcess/WebProcessProxy.cpp

@@ -95 +95 @@
     
     for (size_t i = 0; i < m_pendingMessages.size(); ++i)
-        m_pendingMessages[i].destroy();
+        m_pendingMessages[i].releaseArguments();
 
     if (m_processLauncher) {
@@ -399 +399 @@
 }
 
+void WebProcessProxy::didReceiveInvalidMessage(CoreIPC::Connection*, CoreIPC::MessageID messageID)
+{
+    // We received an invalid message from the web process, invalidate our connection and kill it.
+    m_connection->invalidate();
+
+    terminate();
+}
+
 void WebProcessProxy::didBecomeUnresponsive(ResponsivenessTimer*)
 {

trunk/WebKit2/UIProcess/WebProcessProxy.h

@@ -108 +108 @@
     void didReceiveSyncMessage(CoreIPC::Connection*, CoreIPC::MessageID, CoreIPC::ArgumentDecoder*, CoreIPC::ArgumentEncoder*);
     void didClose(CoreIPC::Connection*);
+    void didReceiveInvalidMessage(CoreIPC::Connection*, CoreIPC::MessageID);
         
     // ResponsivenessTimer::Client

trunk/WebKit2/WebProcess/WebProcess.cpp

@@ -333 +333 @@
 }
 
+void WebProcess::didReceiveInvalidMessage(CoreIPC::Connection*, CoreIPC::MessageID)
+{
+    // We received an invalid message, but since this is from the UI process (which we trust),
+    // we'll let it slide.
+}
+
 } // namespace WebKit

trunk/WebKit2/WebProcess/WebProcess.h

@@ -88 +88 @@
     void didReceiveMessage(CoreIPC::Connection*, CoreIPC::MessageID, CoreIPC::ArgumentDecoder*);
     void didClose(CoreIPC::Connection*);
+    void didReceiveInvalidMessage(CoreIPC::Connection*, CoreIPC::MessageID);
 
     RefPtr<CoreIPC::Connection> m_connection;