AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Adding error checks to SkRBuffer

Browse Source 
BUG=
R=robertphillips@google.com, bsalomon@google.com, reed@google.com

Author: sugoi@chromium.org

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

git-svn-id: http://skia.googlecode.com/svn/trunk@12202 2bbb7eff-a529-9590-31e7-b0007b416f81
This commit is contained in:
commit-bot@chromium.org 2013-11-08 19:22:57 +00:00
parent 9901727f21
commit 8f457e3230
11 changed files with 96 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
include/core/SkFlattenableBuffers.h
View File

@ -144,7 +144,15 @@ public:
return SkData::NewFromMalloc(buffer, len);
}
virtual void validate(bool isValid) {}
/** This function validates that the isValid input parameter is true
* If isValidating() is false, then true is always returned
* If isValidating() is true, then true is returned until validate() is called with isValid
* set to false. When isValid is false, an error flag will be set internally and, from that
* point on, validate() will return false. The error flag cannot be unset.
*
* @param isValid result of a test that is expected to be true
*/
virtual bool validate(bool isValid);
private:
template <typename T> T* readFlattenableT();

3
src/core/SkBuffer.cpp
View File

@ -34,11 +34,12 @@ size_t SkRBuffer::skipToAlign4()
return n;
}
void SkRBufferWithSizeCheck::read(void* buffer, size_t size) {
bool SkRBufferWithSizeCheck::read(void* buffer, size_t size) {
fError = fError || (fPos + size > fStop);
if (!fError && (size > 0)) {
readNoSizeCheck(buffer, size);
}
return !fError;
}
void* SkWBuffer::skip(size_t size)

28
src/core/SkBuffer.h
View File

@ -56,23 +56,31 @@ public:
/** Read the specified number of bytes from the data pointer. If buffer is not
null, copy those bytes into buffer.
*/
virtual void read(void* buffer, size_t size) {
virtual bool read(void* buffer, size_t size) {
if (size) {
this->readNoSizeCheck(buffer, size);
}
return true;
}
const void* skip(size_t size); // return start of skipped data
size_t skipToAlign4();
void* readPtr() { void* ptr; read(&ptr, sizeof(ptr)); return ptr; }
SkScalar readScalar() { SkScalar x; read(&x, 4); return x; }
uint32_t readU32() { uint32_t x; read(&x, 4); return x; }
int32_t readS32() { int32_t x; read(&x, 4); return x; }
uint16_t readU16() { uint16_t x; read(&x, 2); return x; }
int16_t readS16() { int16_t x; read(&x, 2); return x; }
uint8_t readU8() { uint8_t x; read(&x, 1); return x; }
bool readBool() { return this->readU8() != 0; }
bool readPtr(void** ptr) { return read(ptr, sizeof(void*)); }
bool readScalar(SkScalar* x) { return read(x, 4); }
bool readU32(uint32_t* x) { return read(x, 4); }
bool readS32(int32_t* x) { return read(x, 4); }
bool readU16(uint16_t* x) { return read(x, 2); }
bool readS16(int16_t* x) { return read(x, 2); }
bool readU8(uint8_t* x) { return read(x, 1); }
bool readBool(bool* x) {
uint8_t u8;
if (this->readU8(&u8)) {
*x = (u8 != 0);
return true;
}
return false;
}
protected:
void readNoSizeCheck(void* buffer, size_t size);
@ -95,7 +103,7 @@ public:
null and the number of bytes to read does not overflow this object's data,
copy those bytes into buffer.
*/
virtual void read(void* buffer, size_t size) SK_OVERRIDE;
virtual bool read(void* buffer, size_t size) SK_OVERRIDE;
/** Returns whether or not a read operation attempted to read past the end of the data.
*/

4
src/core/SkFlattenableBuffers.cpp
View File

@ -89,6 +89,10 @@ SkXfermode* SkFlattenableReadBuffer::readXfermode() {
return this->readFlattenableT<SkXfermode>();
}
bool SkFlattenableReadBuffer::validate(bool isValid) {
return true;
}
///////////////////////////////////////////////////////////////////////////////
SkFlattenableWriteBuffer::SkFlattenableWriteBuffer() {

20
src/core/SkPath.cpp
View File

@ -2120,8 +2120,9 @@ size_t SkPath::writeToMemory(void* storage) const {
(fSegmentMask << kSegmentMask_SerializationShift) |
(fDirection << kDirection_SerializationShift)
#ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V14_AND_ALL_OTHER_INSTANCES_TOO
| (0x1 << kNewFormat_SerializationShift);
| (0x1 << kNewFormat_SerializationShift)
#endif
;
buffer.write32(packed);
@ -2134,7 +2135,11 @@ size_t SkPath::writeToMemory(void* storage) const {
size_t SkPath::readFromMemory(const void* storage, size_t length) {
SkRBufferWithSizeCheck buffer(storage, length);
uint32_t packed = buffer.readS32();
int32_t packed;
if (!buffer.readS32(&packed)) {
return 0;
}
fIsOval = (packed >> kIsOval_SerializationShift) & 1;
fConvexity = (packed >> kConvexity_SerializationShift) & 0xFF;
fFillType = (packed >> kFillType_SerializationShift) & 0xFF;
@ -2144,18 +2149,21 @@ size_t SkPath::readFromMemory(const void* storage, size_t length) {
bool newFormat = (packed >> kNewFormat_SerializationShift) & 1;
#endif
fPathRef.reset(SkPathRef::CreateFromBuffer(&buffer
SkPathRef* pathRef = SkPathRef::CreateFromBuffer(&buffer
#ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V14_AND_ALL_OTHER_INSTANCES_TOO
, newFormat, packed
#endif
));
buffer.skipToAlign4();
);
size_t sizeRead = 0;
if (buffer.isValid()) {
fPathRef.reset(pathRef);
SkDEBUGCODE(this->validate();)
buffer.skipToAlign4();
sizeRead = buffer.pos();
} else if (NULL != pathRef) {
// If the buffer is not valid, pathRef should be NULL
sk_throw();
}
return sizeRead;
}

32
src/core/SkPathRef.cpp
View File

@ -112,7 +112,11 @@ SkPathRef* SkPathRef::CreateFromBuffer(SkRBuffer* buffer
#ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V14_AND_ALL_OTHER_INSTANCES_TOO
if (newFormat) {
#endif
int32_t packed = buffer->readU32();
int32_t packed;
if (!buffer->readS32(&packed)) {
SkDELETE(ref);
return NULL;
}
ref->fIsFinite = (packed >> kIsFinite_SerializationShift) & 1;
#ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V14_AND_ALL_OTHER_INSTANCES_TOO
@ -121,19 +125,27 @@ SkPathRef* SkPathRef::CreateFromBuffer(SkRBuffer* buffer
}
#endif
ref->fGenerationID = buffer->readU32();
int32_t verbCount = buffer->readS32();
int32_t pointCount = buffer->readS32();
int32_t conicCount = buffer->readS32();
ref->resetToSize(verbCount, pointCount, conicCount);
int32_t verbCount, pointCount, conicCount;
if (!buffer->readU32(&(ref->fGenerationID)) ||
!buffer->readS32(&verbCount) ||
!buffer->readS32(&pointCount) ||
!buffer->readS32(&conicCount)) {
SkDELETE(ref);
return NULL;
}
ref->resetToSize(verbCount, pointCount, conicCount);
SkASSERT(verbCount == ref->countVerbs());
SkASSERT(pointCount == ref->countPoints());
SkASSERT(conicCount == ref->fConicWeights.count());
buffer->read(ref->verbsMemWritable(), verbCount * sizeof(uint8_t));
buffer->read(ref->fPoints, pointCount * sizeof(SkPoint));
buffer->read(ref->fConicWeights.begin(), conicCount * sizeof(SkScalar));
buffer->read(&ref->fBounds, sizeof(SkRect));
if (!buffer->read(ref->verbsMemWritable(), verbCount * sizeof(uint8_t)) ||
!buffer->read(ref->fPoints, pointCount * sizeof(SkPoint)) ||
!buffer->read(ref->fConicWeights.begin(), conicCount * sizeof(SkScalar)) ||
!buffer->read(&ref->fBounds, sizeof(SkRect))) {
SkDELETE(ref);
return NULL;
}
ref->fBoundsIsDirty = false;
return ref;
}

9
src/core/SkRegion.cpp
View File

@ -1138,15 +1138,12 @@ size_t SkRegion::readFromMemory(const void* storage, size_t length) {
SkRegion tmp;
int32_t count;
count = buffer.readS32();
if (count >= 0) {
buffer.read(&tmp.fBounds, sizeof(tmp.fBounds));
if (buffer.readS32(&count) && (count >= 0) && buffer.read(&tmp.fBounds, sizeof(tmp.fBounds))) {
if (count == 0) {
tmp.fRunHead = SkRegion_gRectRunHeadPtr;
} else {
int32_t ySpanCount = buffer.readS32();
int32_t intervalCount = buffer.readS32();
if (buffer.isValid()) {
int32_t ySpanCount, intervalCount;
if (buffer.readS32(&ySpanCount) && buffer.readS32(&intervalCount)) {
tmp.allocateRuns(count, ySpanCount, intervalCount);
buffer.read(tmp.fRunHead->writable_runs(), count * sizeof(RunType));
}

9
src/core/SkValidatingReadBuffer.cpp
View File

@ -20,12 +20,13 @@ SkValidatingReadBuffer::SkValidatingReadBuffer(const void* data, size_t size) :
SkValidatingReadBuffer::~SkValidatingReadBuffer() {
}
void SkValidatingReadBuffer::validate(bool isValid) {
bool SkValidatingReadBuffer::validate(bool isValid) {
if (!fError && !isValid) {
// When an error is found, send the read cursor to the end of the stream
fReader.skip(fReader.available());
fError = true;
}
return !fError;
}
void SkValidatingReadBuffer::setMemory(const void* data, size_t size) {
@ -121,7 +122,7 @@ void SkValidatingReadBuffer::readMatrix(SkMatrix* matrix) {
size_t size = 0;
if (!fError) {
size = matrix->readFromMemory(fReader.peek(), fReader.available());
this->validate((SkAlign4(size) != size) || (0 == size));
this->validate((SkAlign4(size) == size) && (0 != size));
}
if (!fError) {
(void)this->skip(size);
@ -146,7 +147,7 @@ void SkValidatingReadBuffer::readRegion(SkRegion* region) {
size_t size = 0;
if (!fError) {
size = region->readFromMemory(fReader.peek(), fReader.available());
this->validate((SkAlign4(size) != size) || (0 == size));
this->validate((SkAlign4(size) == size) && (0 != size));
}
if (!fError) {
(void)this->skip(size);
@ -157,7 +158,7 @@ void SkValidatingReadBuffer::readPath(SkPath* path) {
size_t size = 0;
if (!fError) {
size = path->readFromMemory(fReader.peek(), fReader.available());
this->validate((SkAlign4(size) != size) || (0 == size));
this->validate((SkAlign4(size) == size) && (0 != size));
}
if (!fError) {
(void)this->skip(size);

2
src/core/SkValidatingReadBuffer.h
View File

@ -60,7 +60,7 @@ public:
// TODO: Implement this (securely) when needed
virtual SkTypeface* readTypeface() SK_OVERRIDE;
virtual void validate(bool isValid) SK_OVERRIDE;
virtual bool validate(bool isValid) SK_OVERRIDE;
private:
bool readArray(void* value, size_t size, size_t elementSize);

17
src/ports/SkFontConfigInterface_direct.cpp
View File

@ -42,15 +42,18 @@ size_t SkFontConfigInterface::FontIdentity::readFromMemory(const void* addr,
size_t size) {
SkRBuffer buffer(addr, size);
fID = buffer.readU32();
fTTCIndex = buffer.readU32();
size_t strLen = buffer.readU32();
int weight = buffer.readU32();
int width = buffer.readU32();
SkFontStyle::Slant slant = (SkFontStyle::Slant)buffer.readU8();
(void)buffer.readU32(&fID);
(void)buffer.readS32(&fTTCIndex);
uint32_t strLen, weight, width;
(void)buffer.readU32(&strLen);
(void)buffer.readU32(&weight);
(void)buffer.readU32(&width);
uint8_t u8;
(void)buffer.readU8(&u8);
SkFontStyle::Slant slant = (SkFontStyle::Slant)u8;
fStyle = SkFontStyle(weight, width, slant);
fString.resize(strLen);
buffer.read(fString.writable_str(), strLen);
(void)buffer.read(fString.writable_str(), strLen);
buffer.skipToAlign4();
return buffer.pos(); // the actual number of bytes read

16
tests/SerializationTest.cpp
View File

@ -109,18 +109,18 @@ static void TestObjectSerialization(T* testObj, skiatest::Reporter* reporter) {
// 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<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);
T obj;
SerializationUtils<T>::Read(buffer, &obj);
REPORTER_ASSERT(reporter, !buffer.validate(true));
// 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));
const unsigned char* peekBefore = static_cast<const unsigned char*>(buffer2.skip(0));
T obj2;
SerializationUtils<T>::Read(buffer2, &obj2);
const unsigned char* peekAfter = static_cast<const unsigned char*>(buffer2.skip(0));
// This should have succeeded, since there are enough bytes to read this
REPORTER_ASSERT(reporter, buffer2.validate(true));
REPORTER_ASSERT(reporter, static_cast<size_t>(peekAfter - peekBefore) == bytesWritten);
TestAlignment(testObj, reporter);