[Bf-blender-cvs] [ff9606ddc4e] blender-v3.4-release: Fix use-after-free of asset catalog data in node add menu

[Julian Eisel]

Commit: ff9606ddc4e2681903e484afd7c16e8f20a8ebc2
Author: Julian Eisel
Date:   Fri Nov 18 17:20:07 2022 +0100
Branches: blender-v3.4-release
https://developer.blender.org/rBff9606ddc4e2681903e484afd7c16e8f20a8ebc2

Fix use-after-free of asset catalog data in node add menu

(Probably requires ASan for a reliable crash.)

Steps to reproduce were:
* Enter Geometry Nodes Workspace
* Press "New" button in the geometry nodes editor header
* Right-click the data-block selector -> "Mark as Asset"
* Change 3D View to Asset Browser
* Create a catalog
* Drag new Geometry Nodes asset into the catalog
* Save the file
* Press Shift+A in the geometry nodes editor

There was a general issue here with keeping catalog pointers around
during the add menu building. The way it does things, catalogs may be
reloaded in between.
Since the Current File asset library isn't loaded in a separate thread,
the use-after-free would always happen in between. For other libraries
it could still happen, but apparently didn't by chance.

===================================================================

M	source/blender/blenkernel/BKE_asset_catalog.hh
M	source/blender/blenkernel/BKE_asset_library.hh
M	source/blender/editors/asset/ED_asset_list.h
M	source/blender/editors/space_node/add_menu_assets.cc

===================================================================

diff --git a/source/blender/blenkernel/BKE_asset_catalog.hh b/source/blender/blenkernel/BKE_asset_catalog.hh
index 73c2e00c4c4..3e1a73c23c4 100644
--- a/source/blender/blenkernel/BKE_asset_catalog.hh
+++ b/source/blender/blenkernel/BKE_asset_catalog.hh
@@ -424,8 +424,14 @@ class AssetCatalogDefinitionFile {
   bool ensure_directory_exists(const CatalogFilePath directory_path) const;
 };
 
-/** Asset Catalog definition, containing a symbolic ID and a path that points to a node in the
- * catalog hierarchy. */
+/**
+ * Asset Catalog definition, containing a symbolic ID and a path that points to a node in the
+ * catalog hierarchy.
+ *
+ * \warning The asset system may reload catalogs, invalidating pointers. Thus it's not recommended
+ *          to store pointers to asset catalogs. Store the #CatalogID instead and do a lookup when
+ *          needed.
+ */
 class AssetCatalog {
  public:
   AssetCatalog() = default;
diff --git a/source/blender/blenkernel/BKE_asset_library.hh b/source/blender/blenkernel/BKE_asset_library.hh
index 2058df71f6a..243c8218509 100644
--- a/source/blender/blenkernel/BKE_asset_library.hh
+++ b/source/blender/blenkernel/BKE_asset_library.hh
@@ -61,6 +61,10 @@ Vector<AssetLibraryReference> all_valid_asset_library_refs();
 
 }  // namespace blender::bke
 
+/**
+ * \warning Catalogs are reloaded, invalidating catalog pointers. Do not store catalog pointers,
+ *          store CatalogIDs instead and lookup the catalog where needed.
+ */
 blender::bke::AssetLibrary *BKE_asset_library_load(const Main *bmain,
                                                    const AssetLibraryReference &library_reference);
 
diff --git a/source/blender/editors/asset/ED_asset_list.h b/source/blender/editors/asset/ED_asset_list.h
index 3d2aaa3bda1..bcd5dbca8d4 100644
--- a/source/blender/editors/asset/ED_asset_list.h
+++ b/source/blender/editors/asset/ED_asset_list.h
@@ -20,6 +20,9 @@ struct wmNotifier;
 /**
  * Invoke asset list reading, potentially in a parallel job. Won't wait until the job is done,
  * and may return earlier.
+ *
+ * \warning: Asset list reading involves an #AS_asset_library_load() call which may reload asset
+ *           library data like catalogs (invalidating pointers). Refer to its warning for details.
  */
 void ED_assetlist_storage_fetch(const struct AssetLibraryReference *library_reference,
                                 const struct bContext *C);
diff --git a/source/blender/editors/space_node/add_menu_assets.cc b/source/blender/editors/space_node/add_menu_assets.cc
index 5458a25d74a..f361f70c265 100644
--- a/source/blender/editors/space_node/add_menu_assets.cc
+++ b/source/blender/editors/space_node/add_menu_assets.cc
@@ -49,7 +49,9 @@ struct LibraryAsset {
 
 struct LibraryCatalog {
   bke::AssetLibrary *library;
-  const bke::AssetCatalog *catalog;
+  /* Catalog pointers are not save to store. Use the catalog ID instead and lookup the catalog when
+   * needed. */
+  const bke::CatalogID catalog_id;
 };
 
 struct AssetItemTree {
@@ -88,7 +90,7 @@ static AssetItemTree build_catalog_tree(const bContext &C, const bNodeTree *node
           const bke::CatalogID &id = item.get_catalog_id();
           bke::AssetCatalog *catalog = library->catalog_service->find_catalog(id);
           catalogs_from_all_libraries.insert_item(*catalog);
-          id_to_catalog_map.add(item.get_catalog_id(), LibraryCatalog{library, catalog});
+          id_to_catalog_map.add(item.get_catalog_id(), LibraryCatalog{library, id});
         });
       }
     }
@@ -118,7 +120,9 @@ static AssetItemTree build_catalog_tree(const bContext &C, const bNodeTree *node
       if (library_catalog == nullptr) {
         return true;
       }
-      assets_per_path.add(library_catalog->catalog->path, LibraryAsset{library_ref, asset});
+      const bke::AssetCatalog *catalog = library_catalog->library->catalog_service->find_catalog(
+          library_catalog->catalog_id);
+      assets_per_path.add(catalog->path, LibraryAsset{library_ref, asset});
       return true;
     });
   }