Fix some memory leaks and a use after free

Also return proper D-Bus errors

Signed-off-by: Sebastian Ramacher <sebastian+dev@ramacher.at>

database-plain.c

@@ -326,26 +326,22 @@ plain_add_bookmark(zathura_database_t* db, const char* file,
     return false;
   }
 
-  char* bmx = g_try_malloc(G_ASCII_DTOSTR_BUF_SIZE);
-  if (bmx == NULL) {
-    return false;
-  }
-
-  char* bmy = g_try_malloc(G_ASCII_DTOSTR_BUF_SIZE);
-  if (bmy == NULL) {
-    g_free(bmx);
-    return false;
-  }
-
   char* name = prepare_filename(file);
   char* val_list[] = {
     g_strdup_printf("%d", bookmark->page),
-    g_ascii_dtostr(bmx, G_ASCII_DTOSTR_BUF_SIZE, bookmark->x),
+    g_try_malloc0(G_ASCII_DTOSTR_BUF_SIZE),
-    g_ascii_dtostr(bmy, G_ASCII_DTOSTR_BUF_SIZE, bookmark->y)
+    g_try_malloc0(G_ASCII_DTOSTR_BUF_SIZE)
   };
+  if (name == NULL || val_list[1] == NULL || val_list[2] == NULL) {
+    g_free(name);
+    for (unsigned int i = 0; i < LENGTH(val_list); ++i) {
+      g_free(val_list[i]);
+    }
+    return false;
+  }
 
-  g_free(bmx);
+  g_ascii_dtostr(val_list[1], G_ASCII_DTOSTR_BUF_SIZE, bookmark->x);
-  g_free(bmy);
+  g_ascii_dtostr(val_list[2], G_ASCII_DTOSTR_BUF_SIZE, bookmark->y);
 
   g_key_file_set_string_list(priv->bookmarks, name, bookmark->id, (const char**)val_list, LENGTH(val_list));
 
@@ -417,7 +413,15 @@ plain_load_bookmarks(zathura_database_t* db, const char* file)
     }
 
     bookmark->id    = g_strdup(keys[i]);
-    char **val_list = g_key_file_get_string_list(priv->bookmarks, name, keys[i], &num_vals, NULL);
+    char** val_list = g_key_file_get_string_list(priv->bookmarks, name, keys[i],
+                                                 &num_vals, NULL);
+
+    if (num_vals != 1 && num_vals != 3) {
+      girara_error("Unexpected number of values.");
+      g_free(bookmark);
+      g_strfreev(val_list);
+      continue;
+    }
 
     bookmark->page = atoi(val_list[0]);
 
@@ -427,8 +431,6 @@ plain_load_bookmarks(zathura_database_t* db, const char* file)
     } else if (num_vals == 1) {
        bookmark->x = DBL_MIN;
        bookmark->y = DBL_MIN;
-    } else {
-      girara_debug("This must be a BUG");
     }
 
     girara_list_append(result, bookmark);

database-sqlite.c

@@ -388,6 +388,10 @@ sqlite_load_bookmarks(zathura_database_t* db, const char* file)
 
   girara_list_t* result = girara_sorted_list_new2((girara_compare_function_t) zathura_bookmarks_compare,
                           (girara_free_function_t) zathura_bookmark_free);
+  if (result != NULL) {
+    sqlite3_finalize(stmt);
+    return NULL;
+  }
 
   while (sqlite3_step(stmt) == SQLITE_ROW) {
     zathura_bookmark_t* bookmark = g_try_malloc0(sizeof(zathura_bookmark_t));
@@ -397,8 +401,8 @@ sqlite_load_bookmarks(zathura_database_t* db, const char* file)
 
     bookmark->id   = g_strdup((const char*) sqlite3_column_text(stmt, 0));
     bookmark->page = sqlite3_column_int(stmt, 1);
-    bookmark->x = sqlite3_column_double(stmt, 2);
+    bookmark->x    = sqlite3_column_double(stmt, 2);
-    bookmark->y = sqlite3_column_double(stmt, 3);
+    bookmark->y    = sqlite3_column_double(stmt, 3);
 
     if (bookmark->page > 1) {
       bookmark->x = bookmark->x == 0.0 ? DBL_MIN : bookmark->x;
@@ -524,8 +528,12 @@ sqlite_load_jumplist(zathura_database_t* db, const char* file)
   }
 
   girara_list_t* jumplist = girara_list_new2(g_free);
-  int res = 0;
+  if (jumplist == NULL) {
+    sqlite3_finalize(stmt);
+    return NULL;
+  }
 
+  int res = 0;
   while ((res = sqlite3_step(stmt)) == SQLITE_ROW) {
     zathura_jump_t* jump = g_try_malloc0(sizeof(zathura_jump_t));
     if (jump == NULL) {

dbus-interface.c

@@ -291,18 +291,40 @@ handle_method_call(GDBusConnection* UNUSED(connection),
     /* get rectangles */
     girara_list_t** rectangles = g_try_malloc0(number_of_pages * sizeof(girara_list_t*));
     if (rectangles == NULL) {
+      g_variant_iter_free(iter);
+      g_variant_iter_free(secondary_iter);
+      g_dbus_method_invocation_return_error(invocation, G_DBUS_ERROR,
+                                            G_DBUS_ERROR_NO_MEMORY,
+                                            "Failed to allocate memory.");
       return;
     }
 
     rectangles[page] = girara_list_new2(g_free);
+    if (rectangles[page] == NULL) {
+      g_free(rectangles);
+      g_variant_iter_free(iter);
+      g_variant_iter_free(secondary_iter);
+      g_dbus_method_invocation_return_error(invocation, G_DBUS_ERROR,
+                                            G_DBUS_ERROR_NO_MEMORY,
+                                            "Failed to allocate memory.");
+      return;
+    }
 
     zathura_rectangle_t temp_rect;
     while (g_variant_iter_loop(iter, "(dddd)", &temp_rect.x1, &temp_rect.x2,
           &temp_rect.y1, &temp_rect.y2)) {
       zathura_rectangle_t* rect = g_try_malloc0(sizeof(zathura_rectangle_t));
       if (rect == NULL) {
-        continue;
+        g_variant_iter_free(iter);
+        g_variant_iter_free(secondary_iter);
+        girara_list_free(rectangles[page]);
+        g_free(rectangles);
+        g_dbus_method_invocation_return_error(invocation, G_DBUS_ERROR,
+                                              G_DBUS_ERROR_NO_MEMORY,
+                                              "Failed to allocate memory.");
+        return;
       }
+
       *rect = temp_rect;
       girara_list_append(rectangles[page], rect);
     }
@@ -322,9 +344,19 @@ handle_method_call(GDBusConnection* UNUSED(connection),
       }
 
       zathura_rectangle_t* rect = g_try_malloc0(sizeof(zathura_rectangle_t));
-      if (rect == NULL) {
+      if (rect == NULL || rectangles[temp_page] == NULL) {
-        continue;
+        g_variant_iter_free(secondary_iter);
+        for (unsigned int p = 0; p != number_of_pages; ++p) {
+          girara_list_free(rectangles[p]);
+        }
+        g_free(rectangles);
+        g_free(rect);
+        g_dbus_method_invocation_return_error(invocation, G_DBUS_ERROR,
+                                              G_DBUS_ERROR_NO_MEMORY,
+                                              "Failed to allocate memory.");
+        return;
       }
+
       *rect = temp_rect;
       girara_list_append(rectangles[temp_page], rect);
     }

plugin.c

@@ -164,6 +164,9 @@ zathura_plugin_manager_load(zathura_plugin_manager_t* plugin_manager)
 
     plugin = g_try_malloc0(sizeof(zathura_plugin_t));
     if (plugin == NULL) {
+      girara_error("Failed to allocate memory for plugin.");
+      g_free(path);
+      g_module_close(handle);
       continue;
     }
 
@@ -301,7 +304,7 @@ plugin_mapping_new(zathura_plugin_manager_t* plugin_manager, const gchar* type,
   }
   GIRARA_LIST_FOREACH_END(plugin_manager->type_plugin_mapping, zathura_type_plugin_mapping_t*, iter, mapping);
 
-  zathura_type_plugin_mapping_t* mapping = g_try_malloc(sizeof(zathura_type_plugin_mapping_t));
+  zathura_type_plugin_mapping_t* mapping = g_try_malloc0(sizeof(zathura_type_plugin_mapping_t));
   if (mapping == NULL) {
     return false;
   }

render.c

@@ -122,14 +122,13 @@ page_cache_init(ZathuraRenderer* renderer, size_t cache_size)
 {
   private_t* priv = GET_PRIVATE(renderer);
 
-  priv->page_cache.size = cache_size;
+  priv->page_cache.size  = cache_size;
-  priv->page_cache.cache = g_try_malloc(cache_size * sizeof(int));
+  priv->page_cache.cache = g_try_malloc0(cache_size * sizeof(int));
   if (priv->page_cache.cache == NULL) {
     return false;
   }
 
   page_cache_invalidate_all(renderer);
-
   return true;
 }
 
@@ -142,6 +141,7 @@ zathura_renderer_new(size_t cache_size)
   ZathuraRenderer* ret = ZATHURA_RENDERER(obj);
 
   if (page_cache_init(ret, cache_size) == false) {
+    g_object_unref(obj);
     return NULL;
   }
 
@@ -646,7 +646,8 @@ render(render_job_t* job, ZathuraRenderRequest* request, ZathuraRenderer* render
   const double width = zathura_page_get_width(page);
 
   const double real_scale = page_calc_height_width(document, height, width,
-                                                   &page_height, &page_width, false);
+                                                   &page_height, &page_width,
+                                                   false);
 
 
   cairo_surface_t* surface = cairo_image_surface_create(CAIRO_FORMAT_RGB24,
@@ -685,7 +686,7 @@ render(render_job_t* job, ZathuraRenderRequest* request, ZathuraRenderer* render
   /* before recoloring, check if we've been aborted */
   if (priv->about_to_close == true || job->aborted == true) {
     girara_debug("Rendering of page %d aborted",
-        zathura_page_get_index(request_priv->page) + 1);
+                 zathura_page_get_index(request_priv->page) + 1);
     remove_job_and_free(job);
     cairo_surface_destroy(surface);
     return true;
@@ -696,12 +697,13 @@ render(render_job_t* job, ZathuraRenderRequest* request, ZathuraRenderer* render
     recolor(priv, page_width, page_height, surface);
   }
 
-  emit_completed_signal_t* ecs = g_try_malloc(sizeof(emit_completed_signal_t));
+  emit_completed_signal_t* ecs = g_try_malloc0(sizeof(emit_completed_signal_t));
   if (ecs == NULL) {
+    cairo_surface_destroy(surface);
     return false;
   }
 
-  ecs->job = job;
+  ecs->job     = job;
   ecs->surface = cairo_surface_reference(surface);
 
   /* emit signal from the main context, i.e. the main thread */

utils.c

@@ -211,8 +211,8 @@ replace_substring(const char* string, const char* old, const char* new)
   size_t new_len = strlen(new);
 
   /* count occurrences */
-  unsigned int count = 0;
+  size_t count = 0;
-  unsigned int i = 0;
+  size_t i     = 0;
 
   for (i = 0; string[i] != '\0'; i++) {
     if (strstr(&string[i], old) == &string[i]) {
@@ -230,9 +230,8 @@ replace_substring(const char* string, const char* old, const char* new)
     return NULL;
   }
 
-  i = 0;
-
   /* replace */
+  i = 0;
   while (*string != '\0') {
     if (strstr(string, old) == string) {
       strncpy(&ret[i], new, new_len);

zathura.c

@@ -1251,14 +1251,14 @@ zathura_jumplist_append_jump(zathura_t* zathura)
 {
   g_return_if_fail(zathura != NULL && zathura->jumplist.list != NULL);
 
-  zathura_jump_t *jump = g_try_malloc(sizeof(zathura_jump_t));
+  zathura_jump_t* jump = g_try_malloc0(sizeof(zathura_jump_t));
   if (jump == NULL) {
     return;
   }
 
   jump->page = 0;
-  jump->x = 0.0;
+  jump->x    = 0.0;
-  jump->y = 0.0;
+  jump->y    = 0.0;
   girara_list_append(zathura->jumplist.list, jump);
 
   if (zathura->jumplist.size == 0) {