Avoid trusting the length encoded in the Snapshot if there is an

runtime/vm/isolate.cc

Index: runtime/vm/isolate.cc
===================================================================
--- runtime/vm/isolate.cc	(revision 10993)
+++ runtime/vm/isolate.cc	(working copy)
@@ -80,26 +80,35 @@
 }
 
 
-static RawInstance* DeserializeMessage(void* data) {
-  // Create a snapshot object using the buffer.
-  const Snapshot* snapshot = Snapshot::SetupFromBuffer(data);
-  ASSERT(snapshot->IsMessageSnapshot());
+bool IsolateMessageHandler::HandleMessage(Message* message) {
+  StartIsolateScope start_scope(isolate_);
+  Zone zone(isolate_);
+  HandleScope handle_scope(isolate_);
 
+  const Snapshot* snapshot = Snapshot::SetupFromBuffer(message->data(),
+                                                       message->len());
+  if (snapshot == NULL || !snapshot->IsMessageSnapshot()) {
+    if (message->IsLocal()) {
+      FATAL("IsolateMessageHandler saw malformed message.  Exiting.");

[siva, 2012/08/22 23:30:39]

Do these really need to be fatal errors?

We could drop this garbage message, set a sticky error and return false, the way
it seems to do it in the code below.

[turnidge, 2012/08/23 18:37:57]

I believe it should be fatal, yes.  Made this UNREACHABLE().

On 2012/08/22 23:30:39, asiva wrote:

+    }
+    delete message;
+    return true;
+  }
+
   // Read object back from the snapshot.
   SnapshotReader reader(snapshot, Isolate::Current());
-  Instance& instance = Instance::Handle();
-  instance ^= reader.ReadObject();
-  return instance.raw();
-}
+  const Object& msg_obj = Object::Handle(reader.ReadObject());
+  if (!msg_obj.IsNull() && !msg_obj.IsInstance()) {
+    if (message->IsLocal()) {
+      FATAL("IsolateMessageHandler saw malformed message.  Exiting.");

[siva, 2012/08/22 23:30:39]

Ditto comment.

[turnidge, 2012/08/23 18:37:57]

This goes away in new revision.

+    }
+    delete message;
+    return true;
+  }
 
+  Instance& msg = Instance::Handle();
+  msg ^= msg_obj.raw();  // Can't use Instance::Cast because may be null.
 
-bool IsolateMessageHandler::HandleMessage(Message* message) {
-  StartIsolateScope start_scope(isolate_);
-  Zone zone(isolate_);
-  HandleScope handle_scope(isolate_);
-
-  const Instance& msg =
-      Instance::Handle(DeserializeMessage(message->data()));
   if (message->IsOOB()) {
     // For now the only OOB messages are Mirrors messages.
     HandleMirrorsMessage(isolate_, message->reply_port(), msg);