< copy of the mail that I've sent to mrok >
Hi,
I found a Null Pointer Dereference in libmirage 3.2.2, specifically in the NRG parser.
I'm writing you directly here and not in the public bug tracker cause I'm not sure if this is a security issue or not.
The content of the file that triggers the bug is the following (12 bytes in hex):
02 80 F4 4B 4E 45 52 4F 00 00 00 04
The issue appears at the memcpy at line 103 (mirage_parser_nrg_build_block_index routine) of images/image-nrg/parser.c:
/* Populate block index */ cur_ptr = self->priv->nrg_data; index = 0; do { blockentry = g_new0(NRGBlockIndexEntry, 1); if (!blockentry) { MIRAGE_DEBUG(self, MIRAGE_DEBUG_WARNING, "%s: failed to allocate memory for block index!\n", __debug__); g_set_error(error, MIRAGE_ERROR, MIRAGE_ERROR_PARSER_ERROR, Q_("Failed to allocate memory for block index!")); return FALSE; } blockentry->offset = (guint64) (cur_ptr - self->priv->nrg_data); memcpy(blockentry->block_id, cur_ptr, 4); //// ISSUE HERE cur_ptr += 4;
==15389== Invalid read of size 4
==15389== at 0x87E2A72: mirage_parser_nrg_build_block_index (parser.c:103)
==15389== by 0x87E2A72: mirage_parser_nrg_load_image (parser.c:1003)
==15389== by 0x4E5E285: mirage_context_load_image (context.c:392)
==15389== by 0x400ED9: main (driver.c:13)
==15389== Address 0x0 is not stack'd, malloc'd or (recently) free'd
The variable cur_ptr is NULL. The root cause is at line 992 (mirage_parser_nrg_load_image routine):
/* Read descriptor data */ self->priv->nrg_data = g_malloc(self->priv->nrg_data_length); mirage_stream_seek(self->priv->nrg_stream, trailer_offset, G_SEEK_SET, NULL); if (mirage_stream_read(self->priv->nrg_stream, self->priv->nrg_data, self->priv->nrg_data_length, NULL) != self->priv->nrg_data_length) { MIRAGE_DEBUG(self, MIRAGE_DEBUG_WARNING, "%s: failed to read descriptor!\n", __debug__); g_set_error(error, MIRAGE_ERROR, MIRAGE_ERROR_IMAGE_FILE_ERROR, Q_("Failed to read descriptor!")); succeeded = FALSE; goto end; }
g_malloc, as opposite of the standard malloc, returns NULL when its argument is 0.
So when self->priv->nrg_data_length is 0 g_malloc returns NULL and generates the bug.
My proposed fix is the following:
--- parser_old.c 2019-08-23 20:28:29.036521000 +0200
+++ parser.c 2019-08-23 20:34:24.236933435 +0200
@@ -988,6 +988,14 @@
is a MTYP block provided */
mirage_disc_set_medium_type(self->priv->disc, MIRAGE_MEDIUM_CD);
/ Read descriptor data /
self->priv->nrg_data = g_malloc(self->priv->nrg_data_length);
mirage_stream_seek(self->priv->nrg_stream, trailer_offset, G_SEEK_SET, NULL);
I hope I was helpful, GG for the code is very clear to read.
Regards,
Andrea Fioraldi
Well formatted patch (SourceForge interpreted the patch as hypertext in the issue text):
Last edit: Andrea Fioraldi 2019-08-24
Thanks for the detailed analysis of the issue and the patch! I have applied and pushed it to our git repository.
Best regards,
Rok