FIX Avoid potential attack vectors with thumbnail generation

silverstripe · Dec 21, 2023 · c398cdd · c398cdd
1 parent eb65ee4
commit c398cdd
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 4 deletions.
diff --git a/_config/config.yml b/_config/config.yml
@@ -52,4 +52,4 @@ SilverStripe\Core\Injector\Injector:
   SilverStripe\AssetAdmin\Model\ThumbnailGenerator.graphql:
     class: SilverStripe\AssetAdmin\Model\ThumbnailGenerator
     properties:
-      Generates: true
+      Generates: false
diff --git a/code/GraphQL/Resolvers/FileTypeResolver.php b/code/GraphQL/Resolvers/FileTypeResolver.php
@@ -90,12 +90,27 @@ public static function resolveFileSmallThumbnail($object)
      */
     public static function resolveFileThumbnail($object)
     {
+        // If we're allowed to generate thumbnails for this file, tell the generator it's allowed to do it.
+        $generator = static::singleton()->getThumbnailGenerator();
+        $idsAllowed = FolderTypeResolver::getIdsAllowedToGenerateThumbnails();
+        $shouldGenerateThumbnail = !empty($idsAllowed) && in_array($object->ID, $idsAllowed);
+        if ($shouldGenerateThumbnail) {
+            $origGenerates = $generator->getGenerates();
+            $generator->setGenerates(true);
+        }
+
         // Make large thumbnail
         $width = AssetAdmin::config()->uninherited('thumbnail_width');
         $height = AssetAdmin::config()->uninherited('thumbnail_height');
-        return static::singleton()
-            ->getThumbnailGenerator()
-            ->generateThumbnailLink($object, $width, $height);
+
+        try {
+            return $generator->generateThumbnailLink($object, $width, $height);
+        } finally {
+            // Make sure to set the generates value back to what it was, regardless of what happens
+            if ($shouldGenerateThumbnail) {
+                $generator->setGenerates($origGenerates);
+            }
+        }
     }
 
     /**

diff --git a/code/GraphQL/Resolvers/FolderTypeResolver.php b/code/GraphQL/Resolvers/FolderTypeResolver.php
@@ -22,6 +22,15 @@
 
 class FolderTypeResolver
 {
+    /**
+     * Any IDs of files which we're allowed to generate thumbnails for.
+     * The intention is to (as much as is feasible) only allow generating thumbnails
+     * during asset admin graphQL requests and not for requests that could be performed
+     * by an attacker.
+     * @internal
+     */
+    private static array $idsAllowedToGenerateThumbnails = [];
+
     /**
      * @param Folder $object
      * @param array $args
@@ -74,6 +83,17 @@ public static function resolveFolderChildren(
         $canViewList = $list->filter('ID', $canViewIDs ?: 0)
             ->limit(null);
 
+        // If we haven't already marked IDs as being okay to generate thumbnails,
+        // and we have a safe limit for the number of children,
+        // mark these children as being okay to generate thumbnails for.
+        if (empty(self::$idsAllowedToGenerateThumbnails)
+            && !empty($args['limit'])
+            && is_numeric($args['limit'])
+            && (int)$args['limit'] <= 100
+        ) {
+            self::$idsAllowedToGenerateThumbnails = $canViewIDs;
+        }
+
         return $canViewList;
     }
 
@@ -182,4 +202,9 @@ public static function sortChildren(array $context): Closure
             return $list;
         };
     }
+
+    public static function getIdsAllowedToGenerateThumbnails(): array
+    {
+        return self::$idsAllowedToGenerateThumbnails;
+    }
 }