I'm investigating how to make the IPC transfer a bit more secure on the

src/core/SkFlattenableSerialization.cpp

 /*
  * Copyright 2013 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkFlattenableSerialization.h"
 
 #include "SkData.h"
 #include "SkFlattenable.h"
+#include "SkImageFilter.h"
 #include "SkOrderedReadBuffer.h"
 #include "SkOrderedWriteBuffer.h"
 
 SkData* SkSerializeFlattenable(SkFlattenable* flattenable) {
     SkOrderedWriteBuffer writer(1024);
     writer.setFlags(SkOrderedWriteBuffer::kCrossProcess_Flag);
     writer.writeFlattenable(flattenable);
     uint32_t size = writer.bytesWritten();
     void* data = sk_malloc_throw(size);
     writer.writeToMemory(data);
     return SkData::NewFromMalloc(data, size);
 }
 
 SkFlattenable* SkDeserializeFlattenable(const void* data, size_t size) {
     SkOrderedReadBuffer buffer(data, size);
     return buffer.readFlattenable();
 }
+
+bool SkValidateImageFilterRec(SkFlattenable* flattenable) {
+    if (!flattenable
+        // This object should always have been sent cross-process,
+        // so it should always have a factory
+        || !flattenable->getFactory()

[scroggo, 2013/08/21 23:25:27]

I'm trying to understand how this could return NULL. It would never have been
deserialized properly if getFactory returned NULL, right?

Isn't it also redundant with isA?

[sugoi1, 2013/08/22 15:41:00]

Well, my original thought was that, if someone would manage to call another
function (not a factory) to construct a malicious object here, then the
getFactory() call would fail. But you're right, since I added the isA, this part
is no longer necessary.

+        // Only SkImageFilter objects are allowed
+        || !flattenable->isA(SkFlattenable::IMAGE_FILTER))

[scroggo, 2013/08/21 23:25:27]

If the unflattening created an SkImageFilter, won't this return true even if it
was wrong to create an SkImageFilter?

[sugoi1, 2013/08/22 15:41:00]

How would I know it was wrong to create it ? I have no other information than
the input stream, AFAIK.

[scroggo, 2013/08/22 17:59:01]

My point is that if you read the stream incorrectly, and created an
SkImageFilter, this check would not tell you that you were wrong. What kind of
error does this avoid?

[sugoi1, 2013/08/22 18:14:53]

When this function is called, it verifies that what's supposed to be an
SkImageFilter object or tree is indeed that and only that. No other
SkFlattenables are allowed to appear inside this tree somehow (in case someone
hijacked the stream to create something else, possible maliciously). The
deserialization does not prevent anyone from creating an SkData object or an
SkRasterizer object, for example.

[scroggo, 2013/08/22 19:10:19]

Ah, okay. Thanks for the explanation.

+        return false;
+
+    // Check that all the inputs are also SkImageFilter objects
+    SkImageFilter* filter = static_cast<SkImageFilter*>(flattenable);
+    int nbInputs = filter->countInputs();
+    for (int i = 0; i < nbInputs; ++i)
+        if (!SkValidateImageFilterRec(filter->getInput(i)))
+            return false;
+
+    return true;
+}
+
+SkImageFilter* SkValidateImageFilter(SkFlattenable* flattenable) {
+    return SkValidateImageFilterRec(flattenable) ?
+           static_cast<SkImageFilter*>(flattenable) : 0;
+}