Checking structure sizes before reading them from memory to avoid overflowing the buffer's stream.

BUG=
R=reed@google.com

Review URL: https://codereview.chromium.org/41253002

git-svn-id: http://skia.googlecode.com/svn/trunk@12114 2bbb7eff-a529-9590-31e7-b0007b416f81

3 files changed, 180 insertions, 118 deletions

diff --git a/tests/MatrixTest.cpp b/tests/MatrixTest.cpp
index 07eacb6f44..e9886941eb 100644
--- a/tests/MatrixTest.cpp
+++ b/tests/MatrixTest.cpp
@@ -112,18 +112,19 @@ static void test_matrix_recttorect(skiatest::Reporter* reporter) {
 
 static void test_flatten(skiatest::Reporter* reporter, const SkMatrix& m) {
     // add 100 in case we have a bug, I don't want to kill my stack in the test
-    char buffer[SkMatrix::kMaxFlattenSize + 100];
-    uint32_t size1 = m.writeToMemory(NULL);
-    uint32_t size2 = m.writeToMemory(buffer);
+    static const size_t kBufferSize = SkMatrix::kMaxFlattenSize + 100;
+    char buffer[kBufferSize];
+    size_t size1 = m.writeToMemory(NULL);
+    size_t size2 = m.writeToMemory(buffer);
     REPORTER_ASSERT(reporter, size1 == size2);
     REPORTER_ASSERT(reporter, size1 <= SkMatrix::kMaxFlattenSize);
 
     SkMatrix m2;
-    uint32_t size3 = m2.readFromMemory(buffer);
+    size_t size3 = m2.readFromMemory(buffer, kBufferSize);
     REPORTER_ASSERT(reporter, size1 == size3);
     REPORTER_ASSERT(reporter, are_equal(reporter, m, m2));
 
-    char buffer2[SkMatrix::kMaxFlattenSize + 100];
+    char buffer2[kBufferSize];
     size3 = m2.writeToMemory(buffer2);
     REPORTER_ASSERT(reporter, size1 == size3);
     REPORTER_ASSERT(reporter, memcmp(buffer, buffer2, size1) == 0);
diff --git a/tests/PathTest.cpp b/tests/PathTest.cpp
index d879ea6e33..f6c2a7ae6e 100644
--- a/tests/PathTest.cpp
+++ b/tests/PathTest.cpp
@@ -1808,12 +1808,12 @@ static void test_flattening(skiatest::Reporter* reporter) {
     // create a buffer that should be much larger than the path so we don't
     // kill our stack if writer goes too far.
     char buffer[1024];
-    uint32_t size1 = p.writeToMemory(NULL);
-    uint32_t size2 = p.writeToMemory(buffer);
+    size_t size1 = p.writeToMemory(NULL);
+    size_t size2 = p.writeToMemory(buffer);
     REPORTER_ASSERT(reporter, size1 == size2);
 
     SkPath p2;
-    uint32_t size3 = p2.readFromMemory(buffer);
+    size_t size3 = p2.readFromMemory(buffer, 1024);
     REPORTER_ASSERT(reporter, size1 == size3);
     REPORTER_ASSERT(reporter, p == p2);
 
diff --git a/tests/SerializationTest.cpp b/tests/SerializationTest.cpp
index d2c692526f..88b9fbbc2e 100644
--- a/tests/SerializationTest.cpp
+++ b/tests/SerializationTest.cpp
@@ -9,140 +9,201 @@
 #include "SkValidatingReadBuffer.h"
 #include "Test.h"
 
-static void Tests(skiatest::Reporter* reporter) {
-    {
-        static const uint32_t arraySize = 512;
-        unsigned char data[arraySize] = {0};
-        SkOrderedWriteBuffer writer(1024);
-        writer.setFlags(SkOrderedWriteBuffer::kValidation_Flag);
-        writer.writeByteArray(data, arraySize);
-        uint32_t bytesWritten = writer.bytesWritten();
-        // This should write the length (in 4 bytes) and the array
-        REPORTER_ASSERT(reporter, (4 + arraySize) == bytesWritten);
+static const uint32_t kArraySize = 64;
+
+template<typename T>
+static void TestAlignment(T* testObj, skiatest::Reporter* reporter) {
+    // Test memory read/write functions directly
+    unsigned char dataWritten[1024];
+    size_t bytesWrittenToMemory = testObj->writeToMemory(dataWritten);
+    REPORTER_ASSERT(reporter, SkAlign4(bytesWrittenToMemory) == bytesWrittenToMemory);
+    size_t bytesReadFromMemory = testObj->readFromMemory(dataWritten, bytesWrittenToMemory);
+    REPORTER_ASSERT(reporter, SkAlign4(bytesReadFromMemory) == bytesReadFromMemory);
+}
 
-        unsigned char dataWritten[1024];
-        writer.writeToMemory(dataWritten);
+template<typename T> struct SerializationUtils {
+};
 
-        // Make sure this fails when it should
-        SkValidatingReadBuffer buffer(dataWritten, bytesWritten);
-        unsigned char dataRead[arraySize];
-        bool success = buffer.readByteArray(dataRead, 256);
-        // This should have failed, since 256 < sizeInBytes
-        REPORTER_ASSERT(reporter, !success);
+template<> struct SerializationUtils<SkMatrix> {
+    static void Write(SkOrderedWriteBuffer& writer, const SkMatrix* matrix) {
+        writer.writeMatrix(*matrix);
+    }
+    static void Read(SkValidatingReadBuffer& reader, SkMatrix* matrix) {
+        reader.readMatrix(matrix);
+    }
+};
 
-        // Make sure this succeeds when it should
-        SkValidatingReadBuffer buffer2(dataWritten, bytesWritten);
-        success = buffer2.readByteArray(dataRead, arraySize);
-        // This should have succeeded, since there are enough bytes to read this
-        REPORTER_ASSERT(reporter, success);
+template<> struct SerializationUtils<SkPath> {
+    static void Write(SkOrderedWriteBuffer& writer, const SkPath* path) {
+        writer.writePath(*path);
     }
+    static void Read(SkValidatingReadBuffer& reader, SkPath* path) {
+        reader.readPath(path);
+    }
+};
 
-    {
-        static const uint32_t arraySize = 64;
-        SkColor data[arraySize];
-        SkOrderedWriteBuffer writer(1024);
-        writer.setFlags(SkOrderedWriteBuffer::kValidation_Flag);
+template<> struct SerializationUtils<SkRegion> {
+    static void Write(SkOrderedWriteBuffer& writer, const SkRegion* region) {
+        writer.writeRegion(*region);
+    }
+    static void Read(SkValidatingReadBuffer& reader, SkRegion* region) {
+        reader.readRegion(region);
+    }
+};
+
+template<> struct SerializationUtils<unsigned char> {
+    static void Write(SkOrderedWriteBuffer& writer, unsigned char* data, uint32_t arraySize) {
+        writer.writeByteArray(data, arraySize);
+    }
+    static bool Read(SkValidatingReadBuffer& reader, unsigned char* data, uint32_t arraySize) {
+        return reader.readByteArray(data, arraySize);
+    }
+};
+
+template<> struct SerializationUtils<SkColor> {
+    static void Write(SkOrderedWriteBuffer& writer, SkColor* data, uint32_t arraySize) {
         writer.writeColorArray(data, arraySize);
-        uint32_t bytesWritten = writer.bytesWritten();
-        // This should write the length (in 4 bytes) and the array
-        REPORTER_ASSERT(reporter, (4 + arraySize * sizeof(SkColor)) == bytesWritten);
+    }
+    static bool Read(SkValidatingReadBuffer& reader, SkColor* data, uint32_t arraySize) {
+        return reader.readColorArray(data, arraySize);
+    }
+};
 
-        unsigned char dataWritten[1024];
-        writer.writeToMemory(dataWritten);
+template<> struct SerializationUtils<int32_t> {
+    static void Write(SkOrderedWriteBuffer& writer, int32_t* data, uint32_t arraySize) {
+        writer.writeIntArray(data, arraySize);
+    }
+    static bool Read(SkValidatingReadBuffer& reader, int32_t* data, uint32_t arraySize) {
+        return reader.readIntArray(data, arraySize);
+    }
+};
 
-        // Make sure this fails when it should
-        SkValidatingReadBuffer buffer(dataWritten, bytesWritten);
-        SkColor dataRead[arraySize];
-        bool success = buffer.readColorArray(dataRead, 32);
-        // This should have failed, since 256 < sizeInBytes
-        REPORTER_ASSERT(reporter, !success);
+template<> struct SerializationUtils<SkPoint> {
+    static void Write(SkOrderedWriteBuffer& writer, SkPoint* data, uint32_t arraySize) {
+        writer.writePointArray(data, arraySize);
+    }
+    static bool Read(SkValidatingReadBuffer& reader, SkPoint* data, uint32_t arraySize) {
+        return reader.readPointArray(data, arraySize);
+    }
+};
 
-        // Make sure this succeeds when it should
-        SkValidatingReadBuffer buffer2(dataWritten, bytesWritten);
-        success = buffer2.readColorArray(dataRead, arraySize);
-        // This should have succeeded, since there are enough bytes to read this
-        REPORTER_ASSERT(reporter, success);
+template<> struct SerializationUtils<SkScalar> {
+    static void Write(SkOrderedWriteBuffer& writer, SkScalar* data, uint32_t arraySize) {
+        writer.writeScalarArray(data, arraySize);
+    }
+    static bool Read(SkValidatingReadBuffer& reader, SkScalar* data, uint32_t arraySize) {
+        return reader.readScalarArray(data, arraySize);
     }
+};
+
+template<typename T>
+static void TestObjectSerialization(T* testObj, skiatest::Reporter* reporter) {
+    SkOrderedWriteBuffer writer(1024);
+    writer.setFlags(SkOrderedWriteBuffer::kValidation_Flag);
+    SerializationUtils<T>::Write(writer, testObj);
+    size_t bytesWritten = writer.bytesWritten();
+
+    unsigned char dataWritten[1024];
+    writer.writeToMemory(dataWritten);
+
+    // Make sure this fails when it should
+    SkValidatingReadBuffer buffer(dataWritten, bytesWritten - 1);
+    const unsigned char* peekBefore = static_cast<const unsigned char*>(buffer.skip(0));
+    SerializationUtils<T>::Read(buffer, testObj);
+    const unsigned char* peekAfter = static_cast<const unsigned char*>(buffer.skip(0));
+    // This should have failed, since the buffer is too small to read a matrix from it
+    REPORTER_ASSERT(reporter, peekBefore == peekAfter);
+
+    // Make sure this succeeds when it should
+    SkValidatingReadBuffer buffer2(dataWritten, bytesWritten);
+    peekBefore = static_cast<const unsigned char*>(buffer2.skip(0));
+    SerializationUtils<T>::Read(buffer2, testObj);
+    peekAfter = static_cast<const unsigned char*>(buffer2.skip(0));
+    // This should have succeeded, since there are enough bytes to read this
+    REPORTER_ASSERT(reporter, static_cast<size_t>(peekAfter - peekBefore) == bytesWritten);
+
+    TestAlignment(testObj, reporter);
+}
+
+template<typename T>
+static void TestArraySerialization(T* data, skiatest::Reporter* reporter) {
+    SkOrderedWriteBuffer writer(1024);
+    writer.setFlags(SkOrderedWriteBuffer::kValidation_Flag);
+    SerializationUtils<T>::Write(writer, data, kArraySize);
+    size_t bytesWritten = writer.bytesWritten();
+    // This should write the length (in 4 bytes) and the array
+    REPORTER_ASSERT(reporter, (4 + kArraySize * sizeof(T)) == bytesWritten);
+
+    unsigned char dataWritten[1024];
+    writer.writeToMemory(dataWritten);
+
+    // Make sure this fails when it should
+    SkValidatingReadBuffer buffer(dataWritten, bytesWritten);
+    T dataRead[kArraySize];
+    bool success = SerializationUtils<T>::Read(buffer, dataRead, kArraySize / 2);
+    // This should have failed, since the provided size was too small
+    REPORTER_ASSERT(reporter, !success);
+
+    // Make sure this succeeds when it should
+    SkValidatingReadBuffer buffer2(dataWritten, bytesWritten);
+    success = SerializationUtils<T>::Read(buffer2, dataRead, kArraySize);
+    // This should have succeeded, since there are enough bytes to read this
+    REPORTER_ASSERT(reporter, success);
+}
 
+static void Tests(skiatest::Reporter* reporter) {
+    // Test matrix serialization
     {
-        static const uint32_t arraySize = 64;
-        int32_t data[arraySize];
-        SkOrderedWriteBuffer writer(1024);
-        writer.setFlags(SkOrderedWriteBuffer::kValidation_Flag);
-        writer.writeIntArray(data, arraySize);
-        uint32_t bytesWritten = writer.bytesWritten();
-        // This should write the length (in 4 bytes) and the array
-        REPORTER_ASSERT(reporter, (4 + arraySize * sizeof(int32_t)) == bytesWritten);
+        SkMatrix matrix = SkMatrix::I();
+        TestObjectSerialization(&matrix, reporter);
+     }
 
-        unsigned char dataWritten[1024];
-        writer.writeToMemory(dataWritten);
+    // Test path serialization
+    {
+        SkPath path;
+        TestObjectSerialization(&path, reporter);
+    }
 
-        // Make sure this fails when it should
-        SkValidatingReadBuffer buffer(dataWritten, bytesWritten);
-        int32_t dataRead[arraySize];
-        bool success = buffer.readIntArray(dataRead, 32);
-        // This should have failed, since 256 < sizeInBytes
-        REPORTER_ASSERT(reporter, !success);
+    // Test region serialization
+    {
+        SkRegion region;
+        TestObjectSerialization(&region, reporter);
+    }
 
-        // Make sure this succeeds when it should
-        SkValidatingReadBuffer buffer2(dataWritten, bytesWritten);
-        success = buffer2.readIntArray(dataRead, arraySize);
-        // This should have succeeded, since there are enough bytes to read this
-        REPORTER_ASSERT(reporter, success);
+    // Test rrect serialization
+    {
+        SkRRect rrect;
+        TestAlignment(&rrect, reporter);
     }
 
+    // Test readByteArray
     {
-        static const uint32_t arraySize = 64;
-        SkPoint data[arraySize];
-        SkOrderedWriteBuffer writer(1024);
-        writer.setFlags(SkOrderedWriteBuffer::kValidation_Flag);
-        writer.writePointArray(data, arraySize);
-        uint32_t bytesWritten = writer.bytesWritten();
-        // This should write the length (in 4 bytes) and the array
-        REPORTER_ASSERT(reporter, (4 + arraySize * sizeof(SkPoint)) == bytesWritten);
+        unsigned char data[kArraySize] = {0};
+        TestArraySerialization(data, reporter);
+    }
 
-        unsigned char dataWritten[1024];
-        writer.writeToMemory(dataWritten);
+    // Test readColorArray
+    {
+        SkColor data[kArraySize];
+        TestArraySerialization(data, reporter);
+    }
 
-        // Make sure this fails when it should
-        SkValidatingReadBuffer buffer(dataWritten, bytesWritten);
-        SkPoint dataRead[arraySize];
-        bool success = buffer.readPointArray(dataRead, 32);
-        // This should have failed, since 256 < sizeInBytes
-        REPORTER_ASSERT(reporter, !success);
+    // Test readIntArray
+    {
+        int32_t data[kArraySize];
+        TestArraySerialization(data, reporter);
+    }
 
-        // Make sure this succeeds when it should
-        SkValidatingReadBuffer buffer2(dataWritten, bytesWritten);
-        success = buffer2.readPointArray(dataRead, arraySize);
-        // This should have succeeded, since there are enough bytes to read this
-        REPORTER_ASSERT(reporter, success);
+    // Test readPointArray
+    {
+        SkPoint data[kArraySize];
+        TestArraySerialization(data, reporter);
     }
 
+    // Test readScalarArray
     {
-        static const uint32_t arraySize = 64;
-        SkScalar data[arraySize];
-        SkOrderedWriteBuffer writer(1024);
-        writer.setFlags(SkOrderedWriteBuffer::kValidation_Flag);
-        writer.writeScalarArray(data, arraySize);
-        uint32_t bytesWritten = writer.bytesWritten();
-        // This should write the length (in 4 bytes) and the array
-        REPORTER_ASSERT(reporter, (4 + arraySize * sizeof(SkScalar)) == bytesWritten);
-
-        unsigned char dataWritten[1024];
-        writer.writeToMemory(dataWritten);
-
-        // Make sure this fails when it should
-        SkValidatingReadBuffer buffer(dataWritten, bytesWritten);
-        SkScalar dataRead[arraySize];
-        bool success = buffer.readScalarArray(dataRead, 32);
-        // This should have failed, since 256 < sizeInBytes
-        REPORTER_ASSERT(reporter, !success);
-
-        // Make sure this succeeds when it should
-        SkValidatingReadBuffer buffer2(dataWritten, bytesWritten);
-        success = buffer2.readScalarArray(dataRead, arraySize);
-        // This should have succeeded, since there are enough bytes to read this
-        REPORTER_ASSERT(reporter, success);
+        SkScalar data[kArraySize];
+        TestArraySerialization(data, reporter);
     }
 }