[webkit-reviews] review denied: [Bug 31302] Add WOFF support for @font-face : [Attachment 62915] Add woffToSfnt() and use it on Mac and Windows

[bugzilla-daemon at webkit.org]

Oliver Hunt <oliver at apple.com> has denied mitz at webkit.org's request for review:
Bug 31302: Add WOFF support for @font-face
https://bugs.webkit.org/show_bug.cgi?id=31302

Attachment 62915: Add woffToSfnt() and use it on Mac and Windows
https://bugs.webkit.org/attachment.cgi?id=62915&action=review

------- Additional Comments from Oliver Hunt <oliver at apple.com>


WebCore/platform/graphics/WOFFToSfnt.cpp:65
 +	if (buffer->size() - offset < sizeof(value))
SharedBuffer::size() and offset are unsigned, if offset is greater than
buffer->size() you overflow and get a very large value

WebCore/platform/graphics/WOFFToSfnt.cpp:77
 +	if (buffer->size() - offset < sizeof(value))
ditto

WebCore/platform/graphics/WOFFToSfnt.cpp:89
 +	if (vector.size() > vector.capacity() - sizeof(value))
Why do you need this check? why not just just use vectors automagic growing?

WebCore/platform/graphics/WOFFToSfnt.cpp:103
 +	    return false;
Same question again

WebCore/platform/graphics/WOFFToSfnt.cpp:150
 +	if (woff->size() - offset < sizeof(uint16_t) + sizeof(uint16_t) +
sizeof(uint32_t) + sizeof(uint32_t) + sizeof(uint32_t) + sizeof(uint32_t) +
sizeof(uint32_t))
Overflow again -- unsigned maths is dangerous if you checking for overflow :-/