extra size check for security

no speed diff observed by removing the test before calling BitWriterResize().

+ remove some unnecessary memset() in VP8LBitWriter
+ fix mixed code/variable-decl in BIG_ENDIAN mode

Change-Id: I36be61f83d10a43e4682b680c2dae0e494da4218
This commit is contained in:
Pascal Massimino 2012-08-01 00:37:24 -07:00
parent 906be65744
commit c19333173a

View File

@ -25,18 +25,23 @@ extern "C" {
static int BitWriterResize(VP8BitWriter* const bw, size_t extra_size) { static int BitWriterResize(VP8BitWriter* const bw, size_t extra_size) {
uint8_t* new_buf; uint8_t* new_buf;
size_t new_size; size_t new_size;
const size_t needed_size = bw->pos_ + extra_size; const uint64_t needed_size_64b = (uint64_t)bw->pos_ + extra_size;
const size_t needed_size = (size_t)needed_size_64b;
if (needed_size_64b != needed_size) {
bw->error_ = 1;
return 0;
}
if (needed_size <= bw->max_pos_) return 1; if (needed_size <= bw->max_pos_) return 1;
// If the following line wraps over 32bit, the test just after will catch it.
new_size = 2 * bw->max_pos_; new_size = 2 * bw->max_pos_;
if (new_size < needed_size) if (new_size < needed_size) new_size = needed_size;
new_size = needed_size;
if (new_size < 1024) new_size = 1024; if (new_size < 1024) new_size = 1024;
new_buf = (uint8_t*)malloc(new_size); new_buf = (uint8_t*)malloc(new_size);
if (new_buf == NULL) { if (new_buf == NULL) {
bw->error_ = 1; bw->error_ = 1;
return 0; return 0;
} }
if (bw->pos_ > 0) memcpy(new_buf, bw->buf_, bw->pos_); memcpy(new_buf, bw->buf_, bw->pos_);
free(bw->buf_); free(bw->buf_);
bw->buf_ = new_buf; bw->buf_ = new_buf;
bw->max_pos_ = new_size; bw->max_pos_ = new_size;
@ -51,10 +56,8 @@ static void kFlush(VP8BitWriter* const bw) {
bw->nb_bits_ -= 8; bw->nb_bits_ -= 8;
if ((bits & 0xff) != 0xff) { if ((bits & 0xff) != 0xff) {
size_t pos = bw->pos_; size_t pos = bw->pos_;
if (pos + bw->run_ >= bw->max_pos_) { // reallocate if (!BitWriterResize(bw, bw->run_ + 1)) {
if (!BitWriterResize(bw, bw->run_ + 1)) { return;
return;
}
} }
if (bits & 0x100) { // overflow -> propagate carry over pending 0xff's if (bits & 0x100) { // overflow -> propagate carry over pending 0xff's
if (pos > 0) bw->buf_[pos - 1]++; if (pos > 0) bw->buf_[pos - 1]++;
@ -194,23 +197,28 @@ void VP8BitWriterWipeOut(VP8BitWriter* const bw) {
static int VP8LBitWriterResize(VP8LBitWriter* const bw, size_t extra_size) { static int VP8LBitWriterResize(VP8LBitWriter* const bw, size_t extra_size) {
uint8_t* allocated_buf; uint8_t* allocated_buf;
size_t allocated_size; size_t allocated_size;
const size_t size_required = VP8LBitWriterNumBytes(bw) + extra_size; const size_t current_size = VP8LBitWriterNumBytes(bw);
if ((bw->max_bytes_ > 0) && (size_required <= bw->max_bytes_)) return 1; const uint64_t size_required_64b = (uint64_t)current_size + extra_size;
allocated_size = (3 * bw->max_bytes_) >> 1; const size_t size_required = (size_t)size_required_64b;
if (allocated_size < size_required) { if (size_required != size_required_64b) {
allocated_size = size_required; bw->error_ = 1;
return 0;
} }
// Make Allocated size multiple of KBs if (bw->max_bytes_ > 0 && size_required <= bw->max_bytes_) return 1;
allocated_size = (3 * bw->max_bytes_) >> 1;
if (allocated_size < size_required) allocated_size = size_required;
// make allocated size multiple of 1k
allocated_size = (((allocated_size >> 10) + 1) << 10); allocated_size = (((allocated_size >> 10) + 1) << 10);
allocated_buf = (uint8_t*)malloc(allocated_size); allocated_buf = (uint8_t*)malloc(allocated_size);
if (allocated_buf == NULL) return 0; if (allocated_buf == NULL) {
memset(allocated_buf, 0, allocated_size); bw->error_ = 1;
if (bw->bit_pos_ > 0) { return 0;
memcpy(allocated_buf, bw->buf_, VP8LBitWriterNumBytes(bw));
} }
memcpy(allocated_buf, bw->buf_, current_size);
free(bw->buf_); free(bw->buf_);
bw->buf_ = allocated_buf; bw->buf_ = allocated_buf;
bw->max_bytes_ = allocated_size; bw->max_bytes_ = allocated_size;
memset(allocated_buf + current_size, 0, allocated_size - current_size);
return 1; return 1;
} }
@ -232,33 +240,37 @@ void VP8LWriteBits(VP8LBitWriter* const bw, int n_bits, uint32_t bits) {
// Technically, this branch of the code can write up to 25 bits at a time, // Technically, this branch of the code can write up to 25 bits at a time,
// but in prefix encoding, the maximum number of bits written is 18 at a time. // but in prefix encoding, the maximum number of bits written is 18 at a time.
{ {
uint8_t* p = &bw->buf_[bw->bit_pos_ >> 3]; uint8_t* const p = &bw->buf_[bw->bit_pos_ >> 3];
uint32_t v = *(const uint32_t*)(p); uint32_t v = *(const uint32_t*)p;
v |= bits << (bw->bit_pos_ & 7); v |= bits << (bw->bit_pos_ & 7);
*(uint32_t*)(p) = v; *(uint32_t*)p = v;
bw->bit_pos_ += n_bits; bw->bit_pos_ += n_bits;
} }
#else // LITTLE_ENDIAN #else // BIG_ENDIAN
// implicit & 0xff is assumed for uint8_t arithmetics
{ {
uint8_t* p = &bw->buf_[bw->bit_pos_ >> 3]; uint8_t* p = &bw->buf_[bw->bit_pos_ >> 3];
const int bits_reserved_in_first_byte = (bw->bit_pos_ & 7); const int bits_reserved_in_first_byte = bw->bit_pos_ & 7;
*p++ |= (bits << bits_reserved_in_first_byte);
const int bits_left_to_write = n_bits - 8 + bits_reserved_in_first_byte; const int bits_left_to_write = n_bits - 8 + bits_reserved_in_first_byte;
// implicit & 0xff is assumed for uint8_t arithmetics
*p++ |= bits << bits_reserved_in_first_byte;
bits >>= 8 - bits_reserved_in_first_byte;
if (bits_left_to_write >= 1) { if (bits_left_to_write >= 1) {
*p++ = bits >> (8 - bits_reserved_in_first_byte); *p++ = bits;
bits >>= 8;
if (bits_left_to_write >= 9) { if (bits_left_to_write >= 9) {
*p++ = bits >> (16 - bits_reserved_in_first_byte); *p++ = bits;
bits >>= 8;
} }
} }
assert(n_bits <= 25); assert(n_bits <= 25);
*p = bits >> (24 - bits_reserved_in_first_byte); *p = bits;
bw->bit_pos_ += n_bits; bw->bit_pos_ += n_bits;
} }
#endif // BIG_ENDIAN #endif
if ((bw->bit_pos_ >> 3) > (bw->max_bytes_ - 8)) { if ((bw->bit_pos_ >> 3) > (bw->max_bytes_ - 8)) {
const size_t kAdditionalBuffer = 32768 + bw->max_bytes_; const uint64_t extra_size = 32768ULL + bw->max_bytes_;
if (!VP8LBitWriterResize(bw, kAdditionalBuffer)) { if (extra_size != (size_t)extra_size ||
!VP8LBitWriterResize(bw, (size_t)extra_size)) {
bw->bit_pos_ = 0; bw->bit_pos_ = 0;
bw->error_ = 1; bw->error_ = 1;
} }