diff options
author | Richard Hughes <richard@hughsie.com> | 2017-12-13 11:59:52 +0000 |
---|---|---|
committer | Richard Hughes <richard@hughsie.com> | 2017-12-14 12:15:12 +0000 |
commit | b4e44c181619e58f16881aa6fbf9d906872bed85 (patch) | |
tree | 48375ea97b2cf5b7b23906f7b57634475119d937 | |
parent | 3197797cf47c32bc048238ca20f78dadbe999a5f (diff) | |
download | gcab-b4e44c181619e58f16881aa6fbf9d906872bed85.tar.gz |
Use g_autoptr() to fix countless memory leaks when parsing corrupt files
Also, fix a memory leak when loading archives with authenticode signatures.
-rw-r--r-- | libgcab/cabinet.c | 31 | ||||
-rw-r--r-- | libgcab/cabinet.h | 31 | ||||
-rw-r--r-- | libgcab/gcab-cabinet.c | 73 | ||||
-rw-r--r-- | libgcab/gcab-file.c | 60 | ||||
-rw-r--r-- | libgcab/gcab-folder.c | 19 | ||||
-rw-r--r-- | libgcab/gcab-priv.h | 4 |
6 files changed, 139 insertions, 79 deletions
diff --git a/libgcab/cabinet.c b/libgcab/cabinet.c index f7b8af3..221a6eb 100644 --- a/libgcab/cabinet.c +++ b/libgcab/cabinet.c @@ -311,6 +311,19 @@ end: return success; } +void +cheader_free (cheader_t *ch) +{ + if (ch == NULL) + return; + g_free (ch->reserved); + g_free (ch->cab_prev); + g_free (ch->disk_prev); + g_free (ch->cab_next); + g_free (ch->disk_next); + g_free (ch); +} + G_GNUC_INTERNAL gboolean cfolder_write (cfolder_t *cf, GDataOutputStream *out, GCancellable *cancellable, GError **error) @@ -350,6 +363,15 @@ end: return success; } +void +cfolder_free (cfolder_t *cf) +{ + if (cf == NULL) + return; + g_free (cf->reserved); + g_free (cf); +} + G_GNUC_INTERNAL gboolean cfile_write (cfile_t *cf, GDataOutputStream *out, GCancellable *cancellable, GError **error) @@ -398,6 +420,15 @@ end: return success; } +void +cfile_free (cfile_t *cf) +{ + if (cf == NULL) + return; + g_free (cf->name); + g_free (cf); +} + static guint32 compute_checksum (guint8 *in, guint16 ncbytes, guint32 seed) { diff --git a/libgcab/cabinet.h b/libgcab/cabinet.h index c0db5ce..0a2b873 100644 --- a/libgcab/cabinet.h +++ b/libgcab/cabinet.h @@ -40,17 +40,12 @@ /* based on the spec http://msdn.microsoft.com/en-us/library/bb417343.aspx */ -typedef struct cheader cheader_t; -typedef struct cfolder cfolder_t; -typedef struct cfile cfile_t; -typedef struct cdata cdata_t; - #define DATABLOCKSIZE 32768 #define CFO_START 0x24 /* folder offset */ #define CFI_START 0x2C /* file offset */ -struct cheader +typedef struct { guint32 res1; guint32 size; @@ -72,7 +67,7 @@ struct cheader gchar *disk_prev; gchar *cab_next; gchar *disk_next; -}; +} cheader_t; typedef enum { CABINET_HEADER_PREV = 0x0001, @@ -80,15 +75,15 @@ typedef enum { CABINET_HEADER_RESERVE = 0x0004, } CabinetHeaderFlags; -struct cfolder +typedef struct { guint32 offsetdata; guint16 ndatab; guint16 typecomp; guint8 *reserved; -}; +} cfolder_t; -struct cfile +typedef struct { guint32 usize; guint32 uoffset; @@ -97,9 +92,9 @@ struct cfile guint16 time; guint16 fattr; gchar *name; -}; +} cfile_t; -struct cdata +typedef struct { guint32 checksum; guint16 ncbytes; @@ -112,7 +107,7 @@ struct cdata /* using wine decomp.h */ FDI_Int fdi; fdi_decomp_state decomp; -}; +} cdata_t; gboolean cheader_write (cheader_t *ch, GDataOutputStream *out, @@ -122,6 +117,8 @@ gboolean cheader_read (cheader_t *ch, GDataInputStream *in, GCancellable *cancellable, GError **error); +void cheader_free (cheader_t *ch); + gboolean cfolder_write (cfolder_t *cf, GDataOutputStream *out, GCancellable *cancellable, @@ -131,6 +128,8 @@ gboolean cfolder_read (cfolder_t *cf, GDataInputStream *in, GCancellable *cancellable, GError **error); +void cfolder_free (cfolder_t *cf); + gboolean cfile_write (cfile_t *cf, GDataOutputStream *out, GCancellable *cancellable, @@ -139,6 +138,8 @@ gboolean cfile_read (cfile_t *cf, GDataInputStream *in, GCancellable *cancellable, GError **error); +void cfile_free (cfile_t *cf); + gboolean cdata_write (cdata_t *cd, GDataOutputStream *out, int type, @@ -155,4 +156,8 @@ gboolean cdata_read (cdata_t *cd, GError **error); void cdata_finish (cdata_t *cd, GError **error); +G_DEFINE_AUTOPTR_CLEANUP_FUNC(cfolder_t, cfolder_free) +G_DEFINE_AUTOPTR_CLEANUP_FUNC(cfile_t, cfile_free) +G_DEFINE_AUTOPTR_CLEANUP_FUNC(cheader_t, cheader_free) + #endif /* CABINET_H */ diff --git a/libgcab/gcab-cabinet.c b/libgcab/gcab-cabinet.c index f2405ae..ebe486e 100644 --- a/libgcab/gcab-cabinet.c +++ b/libgcab/gcab-cabinet.c @@ -41,7 +41,7 @@ struct _GCabCabinet { GPtrArray *folders; GByteArray *reserved; - cheader_t cheader; + cheader_t *cheader; GByteArray *signature; GInputStream *stream; }; @@ -72,6 +72,7 @@ gcab_cabinet_finalize (GObject *object) { GCabCabinet *self = GCAB_CABINET (object); + cheader_free (self->cheader); g_ptr_array_unref (self->folders); if (self->reserved) g_byte_array_unref (self->reserved); @@ -407,57 +408,65 @@ gcab_cabinet_load (GCabCabinet *self, g_return_val_if_fail (self->folders->len == 0, FALSE); g_return_val_if_fail (self->stream == NULL, FALSE); - self->stream = g_object_ref (stream); - - cheader_t cheader; + g_autoptr(cheader_t) cheader = NULL; g_autoptr(GDataInputStream) in = g_data_input_stream_new (stream); g_filter_input_stream_set_close_base_stream (G_FILTER_INPUT_STREAM (in), FALSE); g_data_input_stream_set_byte_order (in, G_DATA_STREAM_BYTE_ORDER_LITTLE_ENDIAN); - if (!cheader_read (&cheader, in, cancellable, error)) + cheader = g_new0 (cheader_t, 1); + if (!cheader_read (cheader, in, cancellable, error)) return FALSE; - if (cheader.reserved) - g_object_set (self, "reserved", - g_byte_array_new_take (cheader.reserved, cheader.res_header), - NULL); + if (cheader->reserved != NULL) { + g_autoptr(GByteArray) blob = NULL; + blob = g_byte_array_new_take (cheader->reserved, cheader->res_header); + g_object_set (self, "reserved", blob, NULL); + cheader->reserved = NULL; + } - for (guint i = 0; i < cheader.nfolders; i++) { - cfolder_t cfolder = { 0, }; - if (!cfolder_read (&cfolder, cheader.res_folder, in, cancellable, error)) + for (guint i = 0; i < cheader->nfolders; i++) { + g_autoptr(cfolder_t) cfolder = g_new0 (cfolder_t, 1); + g_autoptr(GByteArray) blob = NULL; + if (!cfolder_read (cfolder, cheader->res_folder, in, cancellable, error)) return FALSE; - GCabFolder *folder = gcab_folder_new_with_cfolder (&cfolder, stream); - if (cfolder.reserved) - g_object_set (folder, "reserved", - g_byte_array_new_take (cfolder.reserved, cheader.res_folder), - NULL); + /* steal this inelegantly */ + if (cfolder->reserved != NULL) { + blob = g_byte_array_new_take (cfolder->reserved, cheader->res_folder); + cfolder->reserved = NULL; + } + + GCabFolder *folder = gcab_folder_new_steal_cfolder (&cfolder, stream); + if (blob != NULL) + g_object_set (folder, "reserved", blob, NULL); g_ptr_array_add (self->folders, folder); - cfolder.reserved = NULL; } - for (guint i = 0; i < cheader.nfiles; i++) { - cfile_t cfile = { 0, }; - if (!cfile_read (&cfile, in, cancellable, error)) + for (guint i = 0; i < cheader->nfiles; i++) { + g_autoptr(cfile_t) cfile = g_new0 (cfile_t, 1); + if (!cfile_read (cfile, in, cancellable, error)) return FALSE; - if (cfile.index >= self->folders->len) { + if (cfile->index >= self->folders->len) { g_set_error (error, GCAB_ERROR, GCAB_ERROR_FORMAT, "Invalid folder index"); return FALSE; } - GCabFolder *folder = g_ptr_array_index (self->folders, cfile.index); + GCabFolder *folder = g_ptr_array_index (self->folders, cfile->index); if (folder == NULL) { g_set_error (error, GCAB_ERROR, GCAB_ERROR_FORMAT, "Invalid folder pointer"); return FALSE; } - g_autoptr(GCabFile) file = gcab_file_new_with_cfile (&cfile); + g_autoptr(GCabFile) file = gcab_file_new_steal_cfile (&cfile); if (!gcab_folder_add_file (folder, file, FALSE, cancellable, error)) return FALSE; } + + self->stream = g_object_ref (stream); + self->cheader = g_steal_pointer (&cheader); return TRUE; } @@ -486,24 +495,28 @@ gcab_cabinet_extract (GCabCabinet *self, GCancellable *cancellable, GError **error) { - gboolean success = TRUE; - g_return_val_if_fail (GCAB_IS_CABINET (self), FALSE); g_return_val_if_fail (G_IS_FILE (path), FALSE); g_return_val_if_fail (!cancellable || G_IS_CANCELLABLE (cancellable), FALSE); g_return_val_if_fail (!error || *error == NULL, FALSE); + /* never loaded from a stream */ + if (self->cheader == NULL) { + g_set_error (error, GCAB_ERROR, GCAB_ERROR_FAILED, + "Cabinet has not been loaded"); + return FALSE; + } + for (guint i = 0; i < self->folders->len; ++i) { GCabFolder *folder = g_ptr_array_index (self->folders, i); - if (!gcab_folder_extract (folder, path, self->cheader.res_data, + if (!gcab_folder_extract (folder, path, self->cheader->res_data, file_callback, progress_callback, user_data, cancellable, error)) { - success = FALSE; - break; + return FALSE; } } - return success; + return TRUE; } /** diff --git a/libgcab/gcab-file.c b/libgcab/gcab-file.c index ec7c615..11e2368 100644 --- a/libgcab/gcab-file.c +++ b/libgcab/gcab-file.c @@ -45,7 +45,7 @@ struct _GCabFile gchar *extract_name; GFile *file; - cfile_t cfile; + cfile_t *cfile; }; enum { @@ -69,7 +69,7 @@ gcab_file_finalize (GObject *object) if (self->file != NULL) g_object_unref (self->file); - g_free (self->cfile.name); + cfile_free (self->cfile); g_free (self->extract_name); G_OBJECT_CLASS (gcab_file_parent_class)->finalize (object); @@ -80,6 +80,8 @@ gcab_file_set_name (GCabFile *self, const gchar *name) { gchar *fname = g_strdup (name); + g_return_if_fail (self->cfile != NULL); + /* assuming that on win32 we don't get unix paths */ #ifndef G_OS_WIN32 if (fname) { @@ -90,8 +92,8 @@ gcab_file_set_name (GCabFile *self, const gchar *name) } #endif - g_free (self->cfile.name); - self->cfile.name = fname; + g_free (self->cfile->name); + self->cfile->name = fname; } static void @@ -121,7 +123,7 @@ gcab_file_get_property (GObject *object, guint prop_id, GValue *value, GParamSpe switch (prop_id) { case PROP_NAME: - g_value_set_string (value, self->cfile.name); + g_value_set_string (value, self->cfile->name); break; case PROP_FILE: g_value_set_object (value, self->file); @@ -143,12 +145,12 @@ gcab_file_class_init (GCabFileClass *klass) g_object_class_install_property (object_class, PROP_NAME, g_param_spec_string ("name", "name", "name", NULL, - G_PARAM_CONSTRUCT_ONLY | G_PARAM_READWRITE | + G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS)); g_object_class_install_property (object_class, PROP_FILE, g_param_spec_object ("file", "file", "file", G_TYPE_FILE, - G_PARAM_CONSTRUCT_ONLY | G_PARAM_READWRITE | + G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS)); } @@ -166,11 +168,11 @@ gcab_file_update_info (GCabFile *self, GFileInfo *info) time = tv.tv_sec; m = gmtime (&time); - self->cfile.usize = g_file_info_get_size (info); - self->cfile.fattr = GCAB_FILE_ATTRIBUTE_ARCH; - self->cfile.date = ((m->tm_year + 1900 - 1980 ) << 9 ) + + self->cfile->usize = g_file_info_get_size (info); + self->cfile->fattr = GCAB_FILE_ATTRIBUTE_ARCH; + self->cfile->date = ((m->tm_year + 1900 - 1980 ) << 9 ) + ((m->tm_mon+1) << 5 ) + (m->tm_mday); - self->cfile.time = (m->tm_hour << 11) + (m->tm_min << 5) + (m->tm_sec / 2); + self->cfile->time = (m->tm_hour << 11) + (m->tm_min << 5) + (m->tm_sec / 2); return TRUE; } @@ -180,7 +182,7 @@ gcab_file_set_uoffset (GCabFile *self, guint32 uoffset) { g_return_val_if_fail (GCAB_IS_FILE (self), FALSE); - self->cfile.uoffset = uoffset; + self->cfile->uoffset = uoffset; return TRUE; } @@ -189,14 +191,14 @@ G_GNUC_INTERNAL guint32 gcab_file_get_uoffset (GCabFile *self) { g_return_val_if_fail (GCAB_IS_FILE (self), 0); - return self->cfile.uoffset; + return self->cfile->uoffset; } G_GNUC_INTERNAL guint32 gcab_file_get_usize (GCabFile *self) { g_return_val_if_fail (GCAB_IS_FILE (self), 0); - return self->cfile.usize; + return self->cfile->usize; } G_GNUC_INTERNAL GFile * @@ -210,14 +212,14 @@ G_GNUC_INTERNAL void gcab_file_add_attribute (GCabFile *self, guint32 attribute) { g_return_if_fail (GCAB_IS_FILE (self)); - self->cfile.fattr |= attribute; + self->cfile->fattr |= attribute; } G_GNUC_INTERNAL cfile_t * gcab_file_get_cfile (GCabFile *self) { g_return_val_if_fail (GCAB_IS_FILE (self), NULL); - return &self->cfile; + return self->cfile; } /** @@ -234,7 +236,7 @@ gcab_file_get_size (GCabFile *self) { g_return_val_if_fail (GCAB_IS_FILE (self), 0); - return self->cfile.usize; + return self->cfile->usize; } /** @@ -255,8 +257,8 @@ gcab_file_get_date (GCabFile *self, GTimeVal *tv) g_return_if_fail (GCAB_IS_FILE (self)); g_return_if_fail (tv != NULL); - date = self->cfile.date; - time = self->cfile.time; + date = self->cfile->date; + time = self->cfile->time; tm.tm_isdst = -1; tm.tm_year = ((date >> 9) + 1980) - 1900; @@ -285,7 +287,7 @@ gcab_file_get_attributes (GCabFile *self) { g_return_val_if_fail (GCAB_IS_FILE (self), 0); - return self->cfile.fattr; + return self->cfile->fattr; } /** @@ -301,7 +303,7 @@ gcab_file_get_name (GCabFile *self) { g_return_val_if_fail (GCAB_IS_FILE (self), NULL); - return self->cfile.name; + return self->cfile->name; } /** @@ -341,19 +343,21 @@ gcab_file_new_with_file (const gchar *name, GFile *file) g_return_val_if_fail (name != NULL, NULL); g_return_val_if_fail (G_IS_FILE (file), NULL); - return g_object_new (GCAB_TYPE_FILE, - "name", name, - "file", file, - NULL); + GCabFile *self = g_object_new (GCAB_TYPE_FILE, + "file", file, + NULL); + self->cfile = g_new0 (cfile_t, 1); + gcab_file_set_name (self, name); + return self; } G_GNUC_INTERNAL GCabFile * -gcab_file_new_with_cfile (const cfile_t *cfile) +gcab_file_new_steal_cfile (cfile_t **cfile) { g_return_val_if_fail (cfile != NULL, NULL); GCabFile *file = g_object_new (GCAB_TYPE_FILE, NULL); - file->cfile = *cfile; + file->cfile = g_steal_pointer (cfile); return file; } @@ -371,7 +375,7 @@ gcab_file_get_extract_name (GCabFile *self) { g_return_val_if_fail (GCAB_IS_FILE (self), NULL); - return self->extract_name ? self->extract_name : self->cfile.name; + return self->extract_name ? self->extract_name : self->cfile->name; } /** diff --git a/libgcab/gcab-folder.c b/libgcab/gcab-folder.c index 7785fda..fc9b389 100644 --- a/libgcab/gcab-folder.c +++ b/libgcab/gcab-folder.c @@ -53,7 +53,7 @@ struct _GCabFolder GHashTable *hash; gint comptype; GByteArray *reserved; - cfolder_t cfolder; + cfolder_t *cfolder; GInputStream *stream; }; @@ -79,6 +79,7 @@ gcab_folder_finalize (GObject *object) { GCabFolder *self = GCAB_FOLDER (object); + cfolder_free (self->cfolder); g_slist_free_full (self->files, g_object_unref); g_hash_table_unref (self->hash); if (self->reserved) @@ -328,13 +329,16 @@ gcab_folder_new (gint comptype) } G_GNUC_INTERNAL GCabFolder * -gcab_folder_new_with_cfolder (const cfolder_t *folder, GInputStream *stream) +gcab_folder_new_steal_cfolder (cfolder_t **cfolder, GInputStream *stream) { + g_return_val_if_fail (cfolder != NULL, NULL); + g_return_val_if_fail (G_IS_INPUT_STREAM (stream), NULL); + GCabFolder *self = g_object_new (GCAB_TYPE_FOLDER, - "comptype", folder->typecomp, + "comptype", (*cfolder)->typecomp, NULL); self->stream = g_object_ref (stream); - self->cfolder = *folder; + self->cfolder = g_steal_pointer (cfolder); return self; } @@ -383,11 +387,14 @@ gcab_folder_extract (GCabFolder *self, cdata_t cdata = { 0, }; guint32 nubytes = 0; + /* never loaded from a stream */ + g_assert (self->cfolder != NULL); + data = g_data_input_stream_new (self->stream); g_data_input_stream_set_byte_order (data, G_DATA_STREAM_BYTE_ORDER_LITTLE_ENDIAN); g_filter_input_stream_set_close_base_stream (G_FILTER_INPUT_STREAM (data), FALSE); - if (!g_seekable_seek (G_SEEKABLE (data), self->cfolder.offsetdata, G_SEEK_SET, cancellable, error)) + if (!g_seekable_seek (G_SEEKABLE (data), self->cfolder->offsetdata, G_SEEK_SET, cancellable, error)) goto end; files = g_slist_sort (g_slist_copy (self->files), (GCompareFunc)sort_by_offset); @@ -440,7 +447,7 @@ gcab_folder_extract (GCabFolder *self, /* let's rewind if need be */ if (uoffset < nubytes) { - if (!g_seekable_seek (G_SEEKABLE (data), self->cfolder.offsetdata, + if (!g_seekable_seek (G_SEEKABLE (data), self->cfolder->offsetdata, G_SEEK_SET, cancellable, error)) goto end; bzero(&cdata, sizeof(cdata)); diff --git a/libgcab/gcab-priv.h b/libgcab/gcab-priv.h index 6052a43..0cde13d 100644 --- a/libgcab/gcab-priv.h +++ b/libgcab/gcab-priv.h @@ -40,8 +40,8 @@ _GCAB_GET (data, 1, 32, 8) | \ _GCAB_GET (data, 0, 32, 0)) -GCabFolder * gcab_folder_new_with_cfolder (const cfolder_t *folder, GInputStream *stream); -GCabFile * gcab_file_new_with_cfile (const cfile_t *file); +GCabFolder * gcab_folder_new_steal_cfolder (cfolder_t **cfolder, GInputStream *stream); +GCabFile * gcab_file_new_steal_cfile (cfile_t **cfile); gboolean gcab_file_update_info (GCabFile *file, GFileInfo *info); guint32 gcab_file_get_uoffset (GCabFile *file); |