(RPNG) Fix undefined behaviour when loading bad/corrupt PNG images

This commit is contained in:
jdgleaver 2019-05-27 13:04:54 +01:00
parent 277b5c9462
commit 431877799d
6 changed files with 65 additions and 23 deletions

View File

@ -175,7 +175,7 @@ static bool image_texture_load_internal(
if (!img)
goto end;
image_transfer_set_buffer_ptr(img, type, (uint8_t*)ptr);
image_transfer_set_buffer_ptr(img, type, (uint8_t*)ptr, len);
if (!image_transfer_start(img, type))
goto end;

View File

@ -177,13 +177,14 @@ bool image_transfer_is_valid(
void image_transfer_set_buffer_ptr(
void *data,
enum image_type_enum type,
void *ptr)
void *ptr,
size_t len)
{
switch (type)
{
case IMAGE_TYPE_PNG:
#ifdef HAVE_RPNG
rpng_set_buf_ptr((rpng_t*)data, (uint8_t*)ptr);
rpng_set_buf_ptr((rpng_t*)data, (uint8_t*)ptr, len);
#endif
break;
case IMAGE_TYPE_JPEG:

View File

@ -127,6 +127,7 @@ struct rpng
struct idat_buffer idat_buf;
struct png_ihdr ihdr;
uint8_t *buff_data;
uint8_t *buff_end;
uint32_t palette[256];
};
@ -921,20 +922,38 @@ error:
return NULL;
}
static bool read_chunk_header(uint8_t *buf, struct png_chunk *chunk)
static bool read_chunk_header(uint8_t *buf, uint8_t *buf_end, struct png_chunk *chunk)
{
unsigned i;
uint8_t dword[4];
dword[0] = '\0';
/* Check whether reading the header will overflow
* the data buffer */
if (buf_end - buf < 8)
return false;
for (i = 0; i < 4; i++)
dword[i] = buf[i];
chunk->size = dword_be(dword);
/* Check whether chunk will overflow the data buffer */
if (buf + 8 + chunk->size > buf_end)
return false;
for (i = 0; i < 4; i++)
chunk->type[i] = buf[i + 4];
{
uint8_t byte = buf[i + 4];
/* All four bytes of the chunk type must be
* ASCII letters (codes 65-90 and 97-122) */
if ((byte < 65) || ((byte > 90) && (byte < 97)) || (byte > 122))
return false;
chunk->type[i] = byte;
}
return true;
}
@ -968,8 +987,12 @@ bool rpng_iterate_image(rpng_t *rpng)
chunk.type[0] = 0;
chunk.data = NULL;
if (!read_chunk_header(buf, &chunk))
return false;
/* Check whether data buffer pointer is valid */
if (buf > rpng->buff_end)
goto error;
if (!read_chunk_header(buf, rpng->buff_end, &chunk))
goto error;
#if 0
for (i = 0; i < 4; i++)
@ -1072,6 +1095,10 @@ bool rpng_iterate_image(rpng_t *rpng)
rpng->buff_data += chunk.size + 12;
/* Check whether data buffer pointer is valid */
if (rpng->buff_data > rpng->buff_end)
goto error;
return true;
error:
@ -1151,6 +1178,11 @@ bool rpng_start(rpng_t *rpng)
if (!rpng)
return false;
/* Check whether reading the header will overflow
* the data buffer */
if (rpng->buff_end - rpng->buff_data < 8)
return false;
header[0] = '\0';
for (i = 0; i < 8; i++)
@ -1169,21 +1201,21 @@ bool rpng_is_valid(rpng_t *rpng)
if (!rpng)
return false;
if (rpng->has_ihdr)
return true;
if (rpng->has_idat)
return true;
if (rpng->has_iend)
/* A valid PNG image must contain an IHDR chunk,
* one or more IDAT chunks, and an IEND chunk */
if (rpng->has_ihdr && rpng->has_idat && rpng->has_iend)
return true;
return false;
}
bool rpng_set_buf_ptr(rpng_t *rpng, void *data)
bool rpng_set_buf_ptr(rpng_t *rpng, void *data, size_t len)
{
if (!rpng)
if (!rpng || (len < 1))
return false;
rpng->buff_data = (uint8_t*)data;
rpng->buff_end = rpng->buff_data + (len - 1);
return true;
}

View File

@ -84,7 +84,8 @@ bool image_transfer_start(void *data, enum image_type_enum type);
void image_transfer_set_buffer_ptr(
void *data,
enum image_type_enum type,
void *ptr);
void *ptr,
size_t len);
int image_transfer_process(
void *data,

View File

@ -38,7 +38,7 @@ rpng_t *rpng_init(const char *path);
bool rpng_is_valid(rpng_t *rpng);
bool rpng_set_buf_ptr(rpng_t *rpng, void *data);
bool rpng_set_buf_ptr(rpng_t *rpng, void *data, size_t len);
rpng_t *rpng_alloc(void);

View File

@ -88,7 +88,12 @@ static int task_image_process(
unsigned *width,
unsigned *height)
{
int retval = image_transfer_process(
int retval;
if (!image_transfer_is_valid(image->handle, image->type))
return IMAGE_PROCESS_ERROR;
retval = image_transfer_process(
image->handle,
image->type,
&image->ti.pixels, image->size, width, height);
@ -131,8 +136,9 @@ static int task_image_iterate_process_transfer(struct nbio_image_handle *image)
for (i = 0; i < image->processing_pos_increment; i++)
{
if ((retval = task_image_process(image,
&width, &height) != IMAGE_PROCESS_NEXT))
retval = task_image_process(image, &width, &height);
if (retval != IMAGE_PROCESS_NEXT)
break;
}
@ -192,7 +198,7 @@ static int cb_nbio_image_thumbnail(void *data, size_t len)
ptr = nbio_get_ptr(nbio->handle, &len);
image_transfer_set_buffer_ptr(image->handle, image->type, ptr);
image_transfer_set_buffer_ptr(image->handle, image->type, ptr, len);
image->size = len;
image->pos_increment = (len / 2) ?
@ -231,7 +237,8 @@ bool task_image_load_handler(retro_task_t *task)
if (image->handle && image->cb)
{
size_t len = 0;
image->cb(nbio, len);
if (image->cb(nbio, len) == -1)
return false;
}
if (image->is_blocking_on_processing)
image->status = IMAGE_STATUS_PROCESS_TRANSFER;
@ -253,7 +260,8 @@ bool task_image_load_handler(retro_task_t *task)
if (image->handle && image->cb)
{
size_t len = 0;
image->cb(nbio, len);
if (image->cb(nbio, len) == -1)
return false;
}
if (!image->is_finished)
break;
@ -261,7 +269,7 @@ bool task_image_load_handler(retro_task_t *task)
}
if ( nbio->is_finished
&& (image && image->is_finished )
&& (image && image->is_finished)
&& (!task_get_cancelled(task)))
{
struct texture_image *img = (struct texture_image*)malloc(sizeof(struct texture_image));