CVE-2022-29858: Improper Authentication (CWE-287)
value: Silverstripe silverstripe/assets through 1.10 is vulnerable to improper access control that allows protected images to be published by changing an existing image short code on website content.
CWE description: When an actor claims to have a given identity, the product does not prove or insufficiently proves that the claim is correct.
CVSS v3 base score: 4.3
Published: 2022-06-28T22:15Z
Repository: https://github.com/silverstripe/silverstripe-assets
Language: PHP
Files changed: src/Shortcodes/ImageShortcodeProvider.php, tests/php/Shortcodes/ImageShortcodeProviderTest.php
Commit date: 2022-06-28 16:43:15 +1200
Commit message: [CVE-2022-29858] Read grant config for regenerate_shortcode

Diff: diff --git a/src/Shortcodes/ImageShortcodeProvider.php b/src/Shortcodes/ImageShortcodeProvider.php
index e68c7ab..2d5e9ea 100644
--- a/src/Shortcodes/ImageShortcodeProvider.php
+++ b/src/Shortcodes/ImageShortcodeProvider.php
@@ -134,14 +134,16 @@ class ImageShortcodeProvider extends FileShortcodeProvider implements ShortcodeH
      * @param array $extra Extra arguments
      * @return string Result of the handled shortcode
      */
     public static function regenerate_shortcode($args, $content, $parser, $shortcode, $extra = [])
     {
+        $allowSessionGrant = static::config()->allow_session_grant;
+
         // Check if there is a suitable record
         $record = static::find_shortcode_record($args);
         if ($record) {
-            $args['src'] = $record->getURL();
+            $args['src'] = $record->getURL($allowSessionGrant);
         }
 
         // Rebuild shortcode
         $parts = [];
         foreach ($args as $name => $value) {
diff --git a/tests/php/Shortcodes/ImageShortcodeProviderTest.php b/tests/php/Shortcodes/ImageShortcodeProviderTest.php
index be43739..572c207 100644
--- a/tests/php/Shortcodes/ImageShortcodeProviderTest.php
+++ b/tests/php/Shortcodes/ImageShortcodeProviderTest.php
@@ -2,15 +2,21 @@
 
 namespace SilverStripe\Assets\Tests\Shortcodes;
 
 use SilverStripe\Assets\File;
 use Silverstripe\Assets\Dev\TestAssetStore;
+use SilverStripe\Assets\FilenameParsing\ParsedFileID;
+use SilverStripe\Assets\Storage\AssetStore;
 use SilverStripe\Core\Config\Config;
 use SilverStripe\Dev\SapphireTest;
 use SilverStripe\View\Parsers\ShortcodeParser;
 use SilverStripe\Assets\Image;
 use SilverStripe\Assets\Shortcodes\ImageShortcodeProvider;
+use SilverStripe\Assets\Shortcodes\FileShortcodeProvider;
+use SilverStripe\Core\Injector\Injector;
+use SilverStripe\Security\InheritedPermissions;
+use SilverStripe\Security\Member;
 
 /**
  * @skipUpgrade
  */
 class ImageShortcodeProviderTest extends SapphireTest
@@ -185,6 +191,41 @@ class ImageShortcodeProviderTest extends SapphireTest
             // assertion earlier in this function from ImageShortCodeProvider::handle_shortcode()
             $shortcode = '[image id="' . $id . '" width="300" height="200" clear-provider-cache="1"]';
             $this->assertStringNotContainsString('loading="lazy"', $parser->parse($shortcode));
         });
     }
+
+    public function testRegenerateShortcode()
+    {
+        $assetStore = Injector::inst()->get(AssetStore::class);
+        $member = Member::create();
+        $member->write();
+        // Logout first to throw away the existing session which may have image grants.
+        $this->logOut();
+        $this->logInAs($member);
+        // image is in protected asset store
+        $image = $this->objFromFixture(Image::class, 'imageWithTitle');
+        $image->CanViewType = InheritedPermissions::ONLY_THESE_USERS;
+        $image->write();
+        $url = $image->getUrl(false);
+        $args = [
+            'id' => $image->ID,
+            'src' => $url,
+            'width' => '550',
+            'height' => '366',
+            'class' => 'leftAlone ss-htmleditorfield-file image',
+        ];
+        $shortHash = substr($image->getHash(), 0, 10);
+        $expected = implode(' ', [
+            '[image id="' . $image->ID . '" src="/assets/folder/' . $shortHash . '/test-image.png" width="550"',
+            'height="366" class="leftAlone ss-htmleditorfield-file image"]'
+        ]);
+        $parsedFileID = new ParsedFileID($image->getFilename(), $image->getHash());
+        $html = ImageShortcodeProvider::regenerate_shortcode($args, '', '', 'image');
+        $this->assertSame($expected, $html);
+        $this->assertFalse($assetStore->isGranted($parsedFileID));
+        Config::modify()->set(FileShortcodeProvider::class, 'allow_session_grant', true);
+        $html = ImageShortcodeProvider::regenerate_shortcode($args, '', '', 'image');
+        $this->assertSame($expected, $html);
+        $this->assertTrue($assetStore->isGranted($parsedFileID));
+    }
 }

Vulnerable code:             $args['src'] = $record->getURL();
Fixed code:         $allowSessionGrant = static::config()->allow_session_grant;

            $args['src'] = $record->getURL($allowSessionGrant);
use SilverStripe\Assets\FilenameParsing\ParsedFileID;
use SilverStripe\Assets\Storage\AssetStore;
use SilverStripe\Assets\Shortcodes\FileShortcodeProvider;
use SilverStripe\Core\Injector\Injector;
use SilverStripe\Security\InheritedPermissions;
use SilverStripe\Security\Member;

    public function testRegenerateShortcode()
    {
        $assetStore = Injector::inst()->get(AssetStore::class);
        $member = Member::create();
        $member->write();
        // Logout first to throw away the existing session which may have image grants.
        $this->logOut();
        $this->logInAs($member);
        // image is in protected asset store
        $image = $this->objFromFixture(Image::class, 'imageWithTitle');
        $image->CanViewType = InheritedPermissions::ONLY_THESE_USERS;
        $image->write();
        $url = $image->getUrl(false);
        $args = [
            'id' => $image->ID,
            'src' => $url,
            'width' => '550',
            'height' => '366',
            'class' => 'leftAlone ss-htmleditorfield-file image',
        ];
        $shortHash = substr($image->getHash(), 0, 10);
        $expected = implode(' ', [
            '[image id="' . $image->ID . '" src="/assets/folder/' . $shortHash . '/test-image.png" width="550"',
            'height="366" class="leftAlone ss-htmleditorfield-file image"]'
        ]);
        $parsedFileID = new ParsedFileID($image->getFilename(), $image->getHash());
        $html = ImageShortcodeProvider::regenerate_shortcode($args, '', '', 'image');
        $this->assertSame($expected, $html);
        $this->assertFalse($assetStore->isGranted($parsedFileID));
        Config::modify()->set(FileShortcodeProvider::class, 'allow_session_grant', true);
        $html = ImageShortcodeProvider::regenerate_shortcode($args, '', '', 'image');
        $this->assertSame($expected, $html);
        $this->assertTrue($assetStore->isGranted($parsedFileID));
    }