Turn several asserts into formal checks.

There are several places where stb_image protects itself from bad data with
STBI_ASSERT macros, but if these are compiled out in release builds the code
will overflow buffers, etc, without warning. If they are left enabled, the
process will crash from assertion failures.

This patch attempts to leave the assertions in place that are meant to verify
the correctness of the interfaces (if the calling function was meant to pass
only 8 or 16 for bit depth, it's reasonable to assert that is accurate), but
changes asserts that are triggered by corrupt or malicious image file data.

Failed asserts were the majority of crashes during fuzzing; now all of these
cases safely report an error back to the calling app.
This commit is contained in:
Ryan C. Gordon 2020-04-28 10:41:39 -04:00
parent 95560bc6cf
commit 98ca24b8c7

View File

@ -1176,8 +1176,10 @@ static unsigned char *stbi__load_and_postprocess_8bit(stbi__context *s, int *x,
if (result == NULL) if (result == NULL)
return NULL; return NULL;
// it is the responsibility of the loaders to make sure we get either 8 or 16 bit.
STBI_ASSERT(ri.bits_per_channel == 8 || ri.bits_per_channel == 16);
if (ri.bits_per_channel != 8) { if (ri.bits_per_channel != 8) {
STBI_ASSERT(ri.bits_per_channel == 16);
result = stbi__convert_16_to_8((stbi__uint16 *) result, *x, *y, req_comp == 0 ? *comp : req_comp); result = stbi__convert_16_to_8((stbi__uint16 *) result, *x, *y, req_comp == 0 ? *comp : req_comp);
ri.bits_per_channel = 8; ri.bits_per_channel = 8;
} }
@ -1200,8 +1202,10 @@ static stbi__uint16 *stbi__load_and_postprocess_16bit(stbi__context *s, int *x,
if (result == NULL) if (result == NULL)
return NULL; return NULL;
// it is the responsibility of the loaders to make sure we get either 8 or 16 bit.
STBI_ASSERT(ri.bits_per_channel == 8 || ri.bits_per_channel == 16);
if (ri.bits_per_channel != 16) { if (ri.bits_per_channel != 16) {
STBI_ASSERT(ri.bits_per_channel == 8);
result = stbi__convert_8_to_16((stbi_uc *) result, *x, *y, req_comp == 0 ? *comp : req_comp); result = stbi__convert_8_to_16((stbi_uc *) result, *x, *y, req_comp == 0 ? *comp : req_comp);
ri.bits_per_channel = 16; ri.bits_per_channel = 16;
} }
@ -1691,7 +1695,7 @@ static unsigned char *stbi__convert_format(unsigned char *data, int img_n, int r
STBI__CASE(4,1) { dest[0]=stbi__compute_y(src[0],src[1],src[2]); } break; STBI__CASE(4,1) { dest[0]=stbi__compute_y(src[0],src[1],src[2]); } break;
STBI__CASE(4,2) { dest[0]=stbi__compute_y(src[0],src[1],src[2]); dest[1] = src[3]; } break; STBI__CASE(4,2) { dest[0]=stbi__compute_y(src[0],src[1],src[2]); dest[1] = src[3]; } break;
STBI__CASE(4,3) { dest[0]=src[0];dest[1]=src[1];dest[2]=src[2]; } break; STBI__CASE(4,3) { dest[0]=src[0];dest[1]=src[1];dest[2]=src[2]; } break;
default: STBI_ASSERT(0); default: STBI_ASSERT(0); STBI_FREE(data); STBI_FREE(good); return stbi__errpuc("unsupported", "Unsupported format conversion");
} }
#undef STBI__CASE #undef STBI__CASE
} }
@ -1748,7 +1752,7 @@ static stbi__uint16 *stbi__convert_format16(stbi__uint16 *data, int img_n, int r
STBI__CASE(4,1) { dest[0]=stbi__compute_y_16(src[0],src[1],src[2]); } break; STBI__CASE(4,1) { dest[0]=stbi__compute_y_16(src[0],src[1],src[2]); } break;
STBI__CASE(4,2) { dest[0]=stbi__compute_y_16(src[0],src[1],src[2]); dest[1] = src[3]; } break; STBI__CASE(4,2) { dest[0]=stbi__compute_y_16(src[0],src[1],src[2]); dest[1] = src[3]; } break;
STBI__CASE(4,3) { dest[0]=src[0];dest[1]=src[1];dest[2]=src[2]; } break; STBI__CASE(4,3) { dest[0]=src[0];dest[1]=src[1];dest[2]=src[2]; } break;
default: STBI_ASSERT(0); default: STBI_ASSERT(0); STBI_FREE(data); STBI_FREE(good); return (stbi__uint16*) stbi__errpuc("unsupported", "Unsupported format conversion");
} }
#undef STBI__CASE #undef STBI__CASE
} }
@ -2057,7 +2061,7 @@ stbi_inline static int stbi__extend_receive(stbi__jpeg *j, int n)
sgn = (stbi__int32)j->code_buffer >> 31; // sign bit is always in MSB sgn = (stbi__int32)j->code_buffer >> 31; // sign bit is always in MSB
k = stbi_lrot(j->code_buffer, n); k = stbi_lrot(j->code_buffer, n);
STBI_ASSERT(n >= 0 && n < (int) (sizeof(stbi__bmask)/sizeof(*stbi__bmask))); if (n < 0 || n >= (int) (sizeof(stbi__bmask)/sizeof(*stbi__bmask))) return 0;
j->code_buffer = k & ~stbi__bmask[n]; j->code_buffer = k & ~stbi__bmask[n];
k &= stbi__bmask[n]; k &= stbi__bmask[n];
j->code_bits -= n; j->code_bits -= n;
@ -4083,7 +4087,8 @@ static int stbi__zhuffman_decode_slowpath(stbi__zbuf *a, stbi__zhuffman *z)
if (s >= 16) return -1; // invalid code! if (s >= 16) return -1; // invalid code!
// code size is s, so: // code size is s, so:
b = (k >> (16-s)) - z->firstcode[s] + z->firstsymbol[s]; b = (k >> (16-s)) - z->firstcode[s] + z->firstsymbol[s];
STBI_ASSERT(z->size[b] == s); if (b >= sizeof (z->size)) return -1; // some data was corrupt somewhere!
if (z->size[b] != s) return -1; // was originally an assert, but report failure instead.
a->code_buffer >>= s; a->code_buffer >>= s;
a->num_bits -= s; a->num_bits -= s;
return z->value[b]; return z->value[b];
@ -4215,11 +4220,12 @@ static int stbi__compute_huffman_codes(stbi__zbuf *a)
c = stbi__zreceive(a,2)+3; c = stbi__zreceive(a,2)+3;
if (n == 0) return stbi__err("bad codelengths", "Corrupt PNG"); if (n == 0) return stbi__err("bad codelengths", "Corrupt PNG");
fill = lencodes[n-1]; fill = lencodes[n-1];
} else if (c == 17) } else if (c == 17) {
c = stbi__zreceive(a,3)+3; c = stbi__zreceive(a,3)+3;
else { } else if (c == 18) {
STBI_ASSERT(c == 18);
c = stbi__zreceive(a,7)+11; c = stbi__zreceive(a,7)+11;
} else {
return stbi__err("bad codelengths", "Corrupt PNG");
} }
if (ntot - n < c) return stbi__err("bad codelengths", "Corrupt PNG"); if (ntot - n < c) return stbi__err("bad codelengths", "Corrupt PNG");
memset(lencodes+n, fill, c); memset(lencodes+n, fill, c);
@ -4245,7 +4251,7 @@ static int stbi__parse_uncompressed_block(stbi__zbuf *a)
a->code_buffer >>= 8; a->code_buffer >>= 8;
a->num_bits -= 8; a->num_bits -= 8;
} }
STBI_ASSERT(a->num_bits == 0); if (a->num_bits < 0) return stbi__err("zlib corrupt","Corrupt PNG");
// now fill header the normal way // now fill header the normal way
while (k < 4) while (k < 4)
header[k++] = stbi__zget8(a); header[k++] = stbi__zget8(a);
@ -4529,7 +4535,7 @@ static int stbi__create_png_image_raw(stbi__png *a, stbi_uc *raw, stbi__uint32 r
return stbi__err("invalid filter","Corrupt PNG"); return stbi__err("invalid filter","Corrupt PNG");
if (depth < 8) { if (depth < 8) {
STBI_ASSERT(img_width_bytes <= x); if (img_width_bytes > x) return stbi__err("invalid width","Corrupt PNG");
cur += x*out_n - img_width_bytes; // store output to the rightmost img_len bytes, so we can decode in place cur += x*out_n - img_width_bytes; // store output to the rightmost img_len bytes, so we can decode in place
filter_bytes = 1; filter_bytes = 1;
width = img_width_bytes; width = img_width_bytes;
@ -5347,7 +5353,9 @@ static void *stbi__bmp_load(stbi__context *s, int *x, int *y, int *comp, int req
psize = (info.offset - info.extra_read - info.hsz) >> 2; psize = (info.offset - info.extra_read - info.hsz) >> 2;
} }
if (psize == 0) { if (psize == 0) {
STBI_ASSERT(info.offset == (s->img_buffer - s->buffer_start)); if (info.offset != (s->img_buffer - s->buffer_start)) {
return stbi__errpuc("bad offset", "Corrupt BMP");
}
} }
if (info.bpp == 24 && ma == 0xff000000) if (info.bpp == 24 && ma == 0xff000000)