Fix possible NULL pointer dereference in bookmark operators

Although unlikely, the result of BKE_appdir_folder_id_create may be NULL.
Check this for bookmark operators & move writing bookmarks into a shared
utility function.

3 files changed, 63 insertions, 61 deletions

diff --git a/source/blender/editors/space_file/file_ops.c b/source/blender/editors/space_file/file_ops.c
index 23d75c475d9..721c58fc34e 100644
--- a/source/blender/editors/space_file/file_ops.c
+++ b/source/blender/editors/space_file/file_ops.c
@@ -367,6 +367,39 @@ static FileSelect file_select(
 /** \} */
 
 /* -------------------------------------------------------------------- */
+/** \name Bookmark Utilities
+ * \{ */
+
+/**
+ * Local utility to write #BLENDER_BOOKMARK_FILE, reporting an error on failure.
+ */
+static bool fsmenu_write_file_and_refresh_or_report_error(struct FSMenu *fsmenu,
+                                                          ScrArea *area,
+                                                          ReportList *reports)
+{
+  /* NOTE: use warning instead of error here, because the bookmark operation may be part of
+   * other actions which should not cause the operator to fail entirely. */
+  const char *cfgdir = BKE_appdir_folder_id_create(BLENDER_USER_CONFIG, NULL);
+  if (UNLIKELY(!cfgdir)) {
+    BKE_report(reports, RPT_ERROR, "Unable to create configuration directory to write bookmarks");
+    return false;
+  }
+
+  char filepath[FILE_MAX];
+  BLI_join_dirfile(filepath, sizeof(filepath), cfgdir, BLENDER_BOOKMARK_FILE);
+  if (UNLIKELY(!fsmenu_write_file(fsmenu, filepath))) {
+    BKE_reportf(reports, RPT_ERROR, "Unable to open or write bookmark file \"%s\"", filepath);
+    return false;
+  }
+
+  ED_area_tag_refresh(area);
+  ED_area_tag_redraw(area);
+  return true;
+}
+
+/** \} */
+
+/* -------------------------------------------------------------------- */
 /** \name Box Select Operator
  * \{ */
 
@@ -1093,7 +1126,7 @@ void FILE_OT_select_bookmark(wmOperatorType *ot)
 /** \name Add Bookmark Operator
  * \{ */
 
-static int bookmark_add_exec(bContext *C, wmOperator *UNUSED(op))
+static int bookmark_add_exec(bContext *C, wmOperator *op)
 {
   ScrArea *area = CTX_wm_area(C);
   SpaceFile *sfile = CTX_wm_space_file(C);
@@ -1101,19 +1134,11 @@ static int bookmark_add_exec(bContext *C, wmOperator *UNUSED(op))
   struct FileSelectParams *params = ED_fileselect_get_active_params(sfile);
 
   if (params->dir[0] != '\0') {
-    char name[FILE_MAX];
 
     fsmenu_insert_entry(
         fsmenu, FS_CATEGORY_BOOKMARKS, params->dir, NULL, ICON_FILE_FOLDER, FS_INSERT_SAVE);
-    BLI_join_dirfile(name,
-                     sizeof(name),
-                     BKE_appdir_folder_id_create(BLENDER_USER_CONFIG, NULL),
-                     BLENDER_BOOKMARK_FILE);
-    fsmenu_write_file(fsmenu, name);
+    fsmenu_write_file_and_refresh_or_report_error(fsmenu, area, op->reports);
   }
-
-  ED_area_tag_refresh(area);
-  ED_area_tag_redraw(area);
   return OPERATOR_FINISHED;
 }
 
@@ -1147,16 +1172,8 @@ static int bookmark_delete_exec(bContext *C, wmOperator *op)
   const int index = RNA_property_is_set(op->ptr, prop) ? RNA_property_int_get(op->ptr, prop) :
                                                          sfile->bookmarknr;
   if ((index > -1) && (index < nentries)) {
-    char name[FILE_MAX];
-
     fsmenu_remove_entry(fsmenu, FS_CATEGORY_BOOKMARKS, index);
-    BLI_join_dirfile(name,
-                     sizeof(name),
-                     BKE_appdir_folder_id_create(BLENDER_USER_CONFIG, NULL),
-                     BLENDER_BOOKMARK_FILE);
-    fsmenu_write_file(fsmenu, name);
-    ED_area_tag_refresh(area);
-    ED_area_tag_redraw(area);
+    fsmenu_write_file_and_refresh_or_report_error(fsmenu, area, op->reports);
   }
 
   return OPERATOR_FINISHED;
@@ -1187,7 +1204,7 @@ void FILE_OT_bookmark_delete(wmOperatorType *ot)
 /** \name Cleanup Bookmark Operator
  * \{ */
 
-static int bookmark_cleanup_exec(bContext *C, wmOperator *UNUSED(op))
+static int bookmark_cleanup_exec(bContext *C, wmOperator *op)
 {
   ScrArea *area = CTX_wm_area(C);
   struct FSMenu *fsmenu = ED_fsmenu_get();
@@ -1208,16 +1225,8 @@ static int bookmark_cleanup_exec(bContext *C, wmOperator *UNUSED(op))
   }
 
   if (changed) {
-    char name[FILE_MAX];
-
-    BLI_join_dirfile(name,
-                     sizeof(name),
-                     BKE_appdir_folder_id_create(BLENDER_USER_CONFIG, NULL),
-                     BLENDER_BOOKMARK_FILE);
-    fsmenu_write_file(fsmenu, name);
+    fsmenu_write_file_and_refresh_or_report_error(fsmenu, area, op->reports);
     fsmenu_refresh_bookmarks_status(CTX_wm_manager(C), fsmenu);
-    ED_area_tag_refresh(area);
-    ED_area_tag_redraw(area);
   }
 
   return OPERATOR_FINISHED;
@@ -1259,8 +1268,6 @@ static int bookmark_move_exec(bContext *C, wmOperator *op)
   struct FSMenuEntry *fsmentry = ED_fsmenu_get_category(fsmenu, FS_CATEGORY_BOOKMARKS);
   const struct FSMenuEntry *fsmentry_org = fsmentry;
 
-  char fname[FILE_MAX];
-
   const int direction = RNA_enum_get(op->ptr, "direction");
   const int totitems = ED_fsmenu_get_nentries(fsmenu, FS_CATEGORY_BOOKMARKS);
   const int act_index = sfile->bookmarknr;
@@ -1296,13 +1303,8 @@ static int bookmark_move_exec(bContext *C, wmOperator *op)
   /* Need to update active bookmark number. */
   sfile->bookmarknr = new_index;
 
-  BLI_join_dirfile(fname,
-                   sizeof(fname),
-                   BKE_appdir_folder_id_create(BLENDER_USER_CONFIG, NULL),
-                   BLENDER_BOOKMARK_FILE);
-  fsmenu_write_file(fsmenu, fname);
+  fsmenu_write_file_and_refresh_or_report_error(fsmenu, area, op->reports);
 
-  ED_area_tag_redraw(area);
   return OPERATOR_FINISHED;
 }
 
@@ -1342,21 +1344,16 @@ void FILE_OT_bookmark_move(wmOperatorType *ot)
 /** \name Reset Recent Blend Files Operator
  * \{ */
 
-static int reset_recent_exec(bContext *C, wmOperator *UNUSED(op))
+static int reset_recent_exec(bContext *C, wmOperator *op)
 {
   ScrArea *area = CTX_wm_area(C);
-  char name[FILE_MAX];
   struct FSMenu *fsmenu = ED_fsmenu_get();
 
   while (ED_fsmenu_get_entry(fsmenu, FS_CATEGORY_RECENT, 0) != NULL) {
     fsmenu_remove_entry(fsmenu, FS_CATEGORY_RECENT, 0);
   }
-  BLI_join_dirfile(name,
-                   sizeof(name),
-                   BKE_appdir_folder_id_create(BLENDER_USER_CONFIG, NULL),
-                   BLENDER_BOOKMARK_FILE);
-  fsmenu_write_file(fsmenu, name);
-  ED_area_tag_redraw(area);
+
+  fsmenu_write_file_and_refresh_or_report_error(fsmenu, area, op->reports);
 
   return OPERATOR_FINISHED;
 }
@@ -1785,6 +1782,8 @@ static bool file_execute(bContext *C, SpaceFile *sfile)
   }
   /* Opening file, sends events now, so things get handled on window-queue level. */
   else if (sfile->op) {
+    ScrArea *area = CTX_wm_area(C);
+    struct FSMenu *fsmenu = ED_fsmenu_get();
     wmOperator *op = sfile->op;
     char filepath[FILE_MAX];
 
@@ -1793,7 +1792,7 @@ static bool file_execute(bContext *C, SpaceFile *sfile)
     file_sfile_to_operator_ex(bmain, op, sfile, filepath);
 
     if (BLI_exists(params->dir)) {
-      fsmenu_insert_entry(ED_fsmenu_get(),
+      fsmenu_insert_entry(fsmenu,
                           FS_CATEGORY_RECENT,
                           params->dir,
                           NULL,
@@ -1801,11 +1800,8 @@ static bool file_execute(bContext *C, SpaceFile *sfile)
                           FS_INSERT_SAVE | FS_INSERT_FIRST);
     }
 
-    BLI_join_dirfile(filepath,
-                     sizeof(filepath),
-                     BKE_appdir_folder_id_create(BLENDER_USER_CONFIG, NULL),
-                     BLENDER_BOOKMARK_FILE);
-    fsmenu_write_file(ED_fsmenu_get(), filepath);
+    fsmenu_write_file_and_refresh_or_report_error(fsmenu, area, op->reports);
+
     WM_event_fileselect_event(CTX_wm_manager(C), op, EVT_FILESELECT_EXEC);
   }
 
diff --git a/source/blender/editors/space_file/fsmenu.c b/source/blender/editors/space_file/fsmenu.c
index 30e13235f45..35ce7ef364c 100644
--- a/source/blender/editors/space_file/fsmenu.c
+++ b/source/blender/editors/space_file/fsmenu.c
@@ -519,7 +519,7 @@ void fsmenu_remove_entry(struct FSMenu *fsmenu, FSMenuCategory category, int idx
   }
 }
 
-void fsmenu_write_file(struct FSMenu *fsmenu, const char *filepath)
+bool fsmenu_write_file(struct FSMenu *fsmenu, const char *filepath)
 {
   FSMenuEntry *fsm_iter = NULL;
   char fsm_name[FILE_MAX];
@@ -527,33 +527,36 @@ void fsmenu_write_file(struct FSMenu *fsmenu, const char *filepath)
 
   FILE *fp = BLI_fopen(filepath, "w");
   if (!fp) {
-    return;
+    return false;
   }
 
-  fprintf(fp, "[Bookmarks]\n");
+  bool has_error = false;
+  has_error |= (fprintf(fp, "[Bookmarks]\n") < 0);
   for (fsm_iter = ED_fsmenu_get_category(fsmenu, FS_CATEGORY_BOOKMARKS); fsm_iter;
        fsm_iter = fsm_iter->next) {
     if (fsm_iter->path && fsm_iter->save) {
       fsmenu_entry_generate_name(fsm_iter, fsm_name, sizeof(fsm_name));
       if (fsm_iter->name[0] && !STREQ(fsm_iter->name, fsm_name)) {
-        fprintf(fp, "!%s\n", fsm_iter->name);
+        has_error |= (fprintf(fp, "!%s\n", fsm_iter->name) < 0);
       }
-      fprintf(fp, "%s\n", fsm_iter->path);
+      has_error |= (fprintf(fp, "%s\n", fsm_iter->path) < 0);
     }
   }
-  fprintf(fp, "[Recent]\n");
+  has_error = (fprintf(fp, "[Recent]\n") < 0);
   for (fsm_iter = ED_fsmenu_get_category(fsmenu, FS_CATEGORY_RECENT);
        fsm_iter && (nwritten < FSMENU_RECENT_MAX);
        fsm_iter = fsm_iter->next, nwritten++) {
     if (fsm_iter->path && fsm_iter->save) {
       fsmenu_entry_generate_name(fsm_iter, fsm_name, sizeof(fsm_name));
       if (fsm_iter->name[0] && !STREQ(fsm_iter->name, fsm_name)) {
-        fprintf(fp, "!%s\n", fsm_iter->name);
+        has_error |= (fprintf(fp, "!%s\n", fsm_iter->name) < 0);
       }
-      fprintf(fp, "%s\n", fsm_iter->path);
+      has_error |= (fprintf(fp, "%s\n", fsm_iter->path) < 0);
     }
   }
   fclose(fp);
+
+  return !has_error;
 }
 
 void fsmenu_read_bookmarks(struct FSMenu *fsmenu, const char *filepath)
diff --git a/source/blender/editors/space_file/fsmenu.h b/source/blender/editors/space_file/fsmenu.h
index 6e980a326fc..f4f0dafbc73 100644
--- a/source/blender/editors/space_file/fsmenu.h
+++ b/source/blender/editors/space_file/fsmenu.h
@@ -37,8 +37,11 @@ short fsmenu_can_save(struct FSMenu *fsmenu, enum FSMenuCategory category, int i
 /** Removes the fsmenu entry at the given \a index. */
 void fsmenu_remove_entry(struct FSMenu *fsmenu, enum FSMenuCategory category, int idx);
 
-/** saves the 'bookmarks' to the specified file */
-void fsmenu_write_file(struct FSMenu *fsmenu, const char *filepath);
+/**
+ * Saves the 'bookmarks' to the specified file.
+ * \return true on success.
+ */
+bool fsmenu_write_file(struct FSMenu *fsmenu, const char *filepath);
 
 /** reads the 'bookmarks' from the specified file */
 void fsmenu_read_bookmarks(struct FSMenu *fsmenu, const char *filepath);