From 4faa869cdabbdcf4867118b4a1272296baaeeb52 Mon Sep 17 00:00:00 2001 From: "commit-bot@chromium.org" Date: Tue, 5 Nov 2013 15:46:56 +0000 Subject: Checking structure sizes before reading them from memory to avoid overflowing the buffer's stream. BUG= R=reed@google.com, mtklein@google.com, senorblanco@chromium.org Committed: https://code.google.com/p/skia/source/detail?r=12114 Committed: https://code.google.com/p/skia/source/detail?r=12119 Author: sugoi@chromium.org Review URL: https://codereview.chromium.org/41253002 git-svn-id: http://skia.googlecode.com/svn/trunk@12130 2bbb7eff-a529-9590-31e7-b0007b416f81 --- tests/MatrixTest.cpp | 11 +- tests/PathTest.cpp | 6 +- tests/SerializationTest.cpp | 282 +++++++++++++++++++++++++++----------------- 3 files changed, 181 insertions(+), 118 deletions(-) (limited to 'tests') 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..f047ec1284 100644 --- a/tests/SerializationTest.cpp +++ b/tests/SerializationTest.cpp @@ -9,140 +9,202 @@ #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 +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 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 { + 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 { + 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 { + static void Write(SkOrderedWriteBuffer& writer, const SkRegion* region) { + writer.writeRegion(*region); + } + static void Read(SkValidatingReadBuffer& reader, SkRegion* region) { + reader.readRegion(region); + } +}; + +template<> struct SerializationUtils { + 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 { + 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 { + 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 { + 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 { + 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 +static void TestObjectSerialization(T* testObj, skiatest::Reporter* reporter) { + SkOrderedWriteBuffer writer(1024); + writer.setFlags(SkOrderedWriteBuffer::kValidation_Flag); + SerializationUtils::Write(writer, testObj); + size_t bytesWritten = writer.bytesWritten(); + REPORTER_ASSERT(reporter, SkAlign4(bytesWritten) == bytesWritten); + + unsigned char dataWritten[1024]; + writer.writeToMemory(dataWritten); + + // Make sure this fails when it should (test with smaller size, but still multiple of 4) + SkValidatingReadBuffer buffer(dataWritten, bytesWritten - 4); + const unsigned char* peekBefore = static_cast(buffer.skip(0)); + SerializationUtils::Read(buffer, testObj); + const unsigned char* peekAfter = static_cast(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(buffer2.skip(0)); + SerializationUtils::Read(buffer2, testObj); + peekAfter = static_cast(buffer2.skip(0)); + // This should have succeeded, since there are enough bytes to read this + REPORTER_ASSERT(reporter, static_cast(peekAfter - peekBefore) == bytesWritten); + + TestAlignment(testObj, reporter); +} + +template +static void TestArraySerialization(T* data, skiatest::Reporter* reporter) { + SkOrderedWriteBuffer writer(1024); + writer.setFlags(SkOrderedWriteBuffer::kValidation_Flag); + SerializationUtils::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::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::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(®ion, 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); } } -- cgit v1.2.3