Fromr: Stephen Kitt <steve@sk2.org>
Subject: Fix crashes related to invalid CABs

Jakub Wilk pointed out that AFL (http://lcamtuf.coredump.cx/afl/)
finds quite a few crashes caused by invalid CABs. They're mostly due
to either missing input validation or error paths which don't fill in
error information; the attached patch fixes these.

With the patch applied AFL no longer finds any crashes.

Bug-Debian: https://bugs.debian.org/775941
Bug: https://bugzilla.gnome.org/show_bug.cgi?id=743389
Forwarded: https://bug743389.bugzilla-attachments.gnome.org/attachment.cgi?id=295385

--- a/libgcab/gcab-cabinet.c
+++ b/libgcab/gcab-cabinet.c
@@ -415,8 +415,20 @@
         if (!cfile_read (&cfile, in, cancellable, error))
             goto end;
 
-        GCabFile *file = gcab_file_new_with_cfile (&cfile);
+        if (cfile.index >= folders->len) {
+            g_set_error (error, GCAB_ERROR, GCAB_ERROR_FORMAT,
+                         "Invalid folder index");
+            goto end;
+        }
+
         GCabFolder *folder = g_ptr_array_index (folders, cfile.index);
+        if (folder == NULL) {
+            g_set_error (error, GCAB_ERROR, GCAB_ERROR_FORMAT,
+                         "Invalid folder pointer");
+            goto end;
+        }
+
+        GCabFile *file = gcab_file_new_with_cfile (&cfile);
         if (!gcab_folder_add_file (folder, file, FALSE, cancellable, error)) {
             g_object_unref (file);
             goto end;
--- a/libgcab/cabinet.c
+++ b/libgcab/cabinet.c
@@ -96,14 +96,29 @@
 #define RS(val) G_STMT_START{                                   \
     val = _data_input_stream_read_until (in, "\0", 1,           \
                                          cancellable, error);   \
-    if (!val || (error && *error))                              \
+    if (error && *error)                                        \
         goto end;                                               \
+    if (!val) {                                                 \
+        g_set_error(error, GCAB_ERROR, GCAB_ERROR_FORMAT,       \
+                    "Invalid contents");                        \
+        goto end;                                               \
+    }                                                           \
 }G_STMT_END
 #define RN(buff, size) G_STMT_START{                                    \
     if (size) {                                                         \
         gint _val = g_input_stream_read (G_INPUT_STREAM (in), buff, size, cancellable, error); \
-        if (_val == -1 || (error && *error))                            \
+        if (error && *error)                                            \
+            goto end;                                                   \
+        if (_val >= 0 && _val < size) {                                 \
+            g_set_error(error, GCAB_ERROR, GCAB_ERROR_FORMAT,           \
+                        "Expected %d bytes, got %d", size, _val);       \
+            goto end;                                                   \
+        }                                                               \
+        if (_val == -1) {                                               \
+            g_set_error(error, GCAB_ERROR, GCAB_ERROR_FORMAT,           \
+                        "Invalid contents");                            \
             goto end;                                                   \
+        }                                                               \
     }                                                                   \
 }G_STMT_END
 
--- a/libgcab/gcab-folder.c
+++ b/libgcab/gcab-folder.c
@@ -143,10 +143,13 @@
 }
 
 static gboolean
-add_file (GCabFolder *self, GCabFile *file)
+add_file (GCabFolder *self, GCabFile *file, GError **error)
 {
-    if (g_hash_table_lookup (self->hash, (gpointer)gcab_file_get_name (file)))
+    if (g_hash_table_lookup (self->hash, (gpointer)gcab_file_get_name (file))) {
+        g_set_error (error, GCAB_ERROR, GCAB_ERROR_FORMAT,
+                     "File '%s' has already been added", gcab_file_get_name (file));
         return FALSE;
+    }
 
     g_hash_table_insert (self->hash,
                          (gpointer)gcab_file_get_name (file), g_object_ref (file));
@@ -196,7 +199,7 @@
 
     } else if (file_type == G_FILE_TYPE_REGULAR) {
         gcab_file_update_info (file, info);
-        if (!add_file (self, file))
+        if (!add_file (self, file, error))
             return FALSE;
 
     } else {
@@ -244,7 +247,7 @@
                                  gcab_file_get_name (file), recurse, error);
         g_object_unref (info);
     } else {
-        success = add_file (self, file);
+        success = add_file (self, file, error);
     }
 
     return success;
--- a/gcab.c
+++ b/gcab.c
@@ -140,7 +140,7 @@
     g_option_context_add_main_entries (context, entries, GETTEXT_PACKAGE);
     g_option_context_set_translation_domain (context, GETTEXT_PACKAGE);
     if (!g_option_context_parse (context, &argc, &argv, &error))
-        gcab_error (_("option parsing failed: %s\n"), error->message);
+        gcab_error (_("option parsing failed: %s\n"), (error && error->message) ? error->message : "unknown error");
     g_option_context_free(context);
 
     if (version) {
@@ -166,9 +166,9 @@
         GInputStream *in = G_INPUT_STREAM (g_file_read (file, cancellable, &error));
 
         if (!in)
-            gcab_error (_("can't open %s for reading: %s\n"), args[0], error->message);
+            gcab_error (_("can't open %s for reading: %s\n"), args[0], (error && error->message) ? error->message : "unknown error");
         if (!gcab_cabinet_load (cabinet, in, cancellable, &error))
-            gcab_error (_("error reading %s: %s\n"), args[0], error->message);
+            gcab_error (_("error reading %s: %s\n"), args[0], (error && error->message) ? error->message : "unknown error");
 
         if (list) {
             GPtrArray *folders = gcab_cabinet_get_folders (cabinet);
@@ -185,7 +185,7 @@
             file = g_file_new_for_path (change);
 
             if (!gcab_cabinet_extract (cabinet, file, file_callback, NULL, NULL, cancellable, &error))
-                gcab_error (_("error during extraction: %s"), error->message);
+                gcab_error (_("error during extraction: %s"), (error && error->message) ? error->message : "unknown error");
         } else if (dump_reserved) {
             GByteArray *reserved;
 
@@ -197,7 +197,7 @@
 
             reserved = (GByteArray *)gcab_cabinet_get_signature (cabinet, cancellable, &error);
             if (error)
-                gcab_error (_("error while reading signature: %s"), error->message);
+                gcab_error (_("error while reading signature: %s"), (error && error->message) ? error->message : "unknown error");
             if (reserved != NULL)
                 save_array_to_file (reserved, args[0], "signature");
         }
@@ -226,7 +226,7 @@
                                  remove_leading_path (name), file);
 
         if (!gcab_folder_add_file (folder, cabfile, TRUE, NULL, &error)) {
-            g_warning (_("Can't add file %s: %s"), args[i], error->message);
+            g_warning (_("Can't add file %s: %s"), args[i], (error && error->message) ? error->message : "unknown error");
             g_clear_error (&error);
         }
 
@@ -242,11 +242,11 @@
     output = G_OUTPUT_STREAM (g_file_replace (outputfile, NULL, FALSE,
                                               0, NULL, &error));
     if (error)
-        gcab_error (_("can't create cab file %s: %s"), args[0], error->message);
+        gcab_error (_("can't create cab file %s: %s"), args[0], (error && error->message) ? error->message : "unknown error");
 
     cwd = g_file_new_for_commandline_arg (".");
     if (!gcab_cabinet_add_folder (cabinet, folder, &error))
-        gcab_error (_("can't add folder to cab file %s: %s"), args[0], error->message);
+        gcab_error (_("can't add folder to cab file %s: %s"), args[0], (error && error->message) ? error->message : "unknown error");
 
     if (!gcab_cabinet_write (cabinet, output,
                              file_callback,
@@ -254,7 +254,7 @@
                              cwd,
                              NULL,
                              &error))
-        gcab_error (_("can't write cab file %s: %s"), args[0], error->message);
+        gcab_error (_("can't write cab file %s: %s"), args[0], (error && error->message) ? error->message : "unknown error");
 
     g_object_unref (cwd);
     g_object_unref (output);
