CVE-2024-25737: Insufficient Information (NVD-CWE-noinfo)
value: A Server-Side Request Forgery (SSRF) vulnerability in the /Cover/Show route (showAction in CoverController.php) in Open Library Foundation VuFind 2.4 through 9.1 before 9.1.1 allows remote attackers to access internal HTTP servers and perform Cross-Site Scripting (XSS) attacks by proxying arbitrary URLs via the proxy GET parameter.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
Published: 2024-05-22T19:15Z
Repository: https://github.com/vufind-org/vufind
Language: PHP
Files changed: config/vufind/config.ini, module/VuFind/src/VuFind/Controller/CoverController.php, module/VuFind/src/VuFind/Controller/CoverControllerFactory.php
Commit date: 2024-02-06 08:16:58 -0500
Commit message: Restrict hostnames for cover proxy. (#3385)


Diff: diff --git a/config/vufind/config.ini b/config/vufind/config.ini
index 1feffcb82..6fcbcc1da 100644
--- a/config/vufind/config.ini
+++ b/config/vufind/config.ini
@@ -1021,10 +1021,17 @@ coverimagesCache = true
 ; "/.*/" to turn on caching for all proxied images.
 ;coverproxyCache[] = "/.*\.?mylibrary\.edu/"
 ; BrowZine DOI icons:
 coverproxyCache[] = "/assets\.thirdiron\.com/"
 
+; This setting controls which hosts are allowed to provide cover images through
+; the built-in cover proxy. Images from hostnames that do not match the regular
+; expressions below will not be proxied. Be sure to define regular expressions
+; using an end of string ($) marker to prevent abuse.
+coverproxyAllowedHosts[] = "/assets\.thirdiron\.com$/"
+coverproxyAllowedHosts[] = "/\.summon\.serialssolutions\.com$/"
+
 ; This setting controls how cover image URLs are loaded. They could be loaded as
 ; part of main request, or asynchronously. Asynchronous loading is disabled by
 ; default; to enable it, just uncomment the line below.
 ;ajaxcovers = true
 
diff --git a/module/VuFind/src/VuFind/Controller/CoverController.php b/module/VuFind/src/VuFind/Controller/CoverController.php
index 3530902db..49b6e952a 100644
--- a/module/VuFind/src/VuFind/Controller/CoverController.php
+++ b/module/VuFind/src/VuFind/Controller/CoverController.php
@@ -63,25 +63,35 @@ class CoverController extends \Laminas\Mvc\Controller\AbstractActionController
      *
      * @var SessionSettings
      */
     protected $sessionSettings = null;
 
+    /**
+     * Configuration settings ([Content] section of config.ini)
+     *
+     * @var array
+     */
+    protected $config;
+
     /**
      * Constructor
      *
      * @param Loader          $loader Cover loader
      * @param CachingProxy    $proxy  Proxy loader
      * @param SessionSettings $ss     Session settings
+     * @param array           $config Configuration settings
      */
     public function __construct(
         Loader $loader,
         CachingProxy $proxy,
-        SessionSettings $ss
+        SessionSettings $ss,
+        array $config = []
     ) {
         $this->loader = $loader;
         $this->proxy = $proxy;
         $this->sessionSettings = $ss;
+        $this->config = $config;
     }
 
     /**
      * Convert image parameters into an array for use by the image loader.
      *
@@ -113,10 +123,31 @@ class CoverController extends \Laminas\Mvc\Controller\AbstractActionController
             'nbn' => $params()->fromQuery('nbn'),
             'ismn' => $params()->fromQuery('ismn'),
         ];
     }
 
+    /**
+     * Is the provided URL included on the configured allow list?
+     *
+     * @param string $url URL to check
+     *
+     * @return bool
+     */
+    protected function proxyAllowedForUrl(string $url): bool
+    {
+        $host = parse_url($url, PHP_URL_HOST);
+        if (!$host) {
+            return false;
+        }
+        foreach ((array)($this->config['coverproxyAllowedHosts'] ?? []) as $regEx) {
+            if (preg_match($regEx, $host)) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     /**
      * Send image data for display in the view
      *
      * @return \Laminas\Http\Response
      */
@@ -124,11 +155,11 @@ class CoverController extends \Laminas\Mvc\Controller\AbstractActionController
     {
         $this->sessionSettings->disableWrite(); // avoid session write timing bug
 
         // Special case: proxy a full URL:
         $url = $this->params()->fromQuery('proxy');
-        if (!empty($url)) {
+        if (!empty($url) && $this->proxyAllowedForUrl($url)) {
             try {
                 $image = $this->proxy->fetch($url);
                 $contentType = $image?->getHeaders()?->get('content-type')?->getFieldValue() ?? '';
                 if (str_starts_with(strtolower($contentType), 'image/')) {
                     return $this->displayImage(
diff --git a/module/VuFind/src/VuFind/Controller/CoverControllerFactory.php b/module/VuFind/src/VuFind/Controller/CoverControllerFactory.php
index 14217bb21..ea0aa3ac5 100644
--- a/module/VuFind/src/VuFind/Controller/CoverControllerFactory.php
+++ b/module/VuFind/src/VuFind/Controller/CoverControllerFactory.php
@@ -66,12 +66,15 @@ class CoverControllerFactory implements FactoryInterface
         array $options = null
     ) {
         if (!empty($options)) {
             throw new \Exception('Unexpected options sent to factory.');
         }
+        $config = $container->get(\VuFind\Config\PluginManager::class)->get('config');
+        $configArray = $config?->Content?->toArray() ?? [];
         return new $requestedName(
             $container->get(\VuFind\Cover\Loader::class),
             $container->get(\VuFind\Cover\CachingProxy::class),
-            $container->get(\VuFind\Session\Settings::class)
+            $container->get(\VuFind\Session\Settings::class),
+            $configArray
         );
     }
 }

Vulnerable code:         SessionSettings $ss
        if (!empty($url)) {
            $container->get(\VuFind\Session\Settings::class)
Fixed code: ; This setting controls which hosts are allowed to provide cover images through
; the built-in cover proxy. Images from hostnames that do not match the regular
; expressions below will not be proxied. Be sure to define regular expressions
; using an end of string ($) marker to prevent abuse.
coverproxyAllowedHosts[] = "/assets\.thirdiron\.com$/"
coverproxyAllowedHosts[] = "/\.summon\.serialssolutions\.com$/"

    /**
     * Configuration settings ([Content] section of config.ini)
     *
     * @var array
     */
    protected $config;

     * @param array           $config Configuration settings
        SessionSettings $ss,
        array $config = []
        $this->config = $config;
    /**
     * Is the provided URL included on the configured allow list?
     *
     * @param string $url URL to check
     *
     * @return bool
     */
    protected function proxyAllowedForUrl(string $url): bool
    {
        $host = parse_url($url, PHP_URL_HOST);
        if (!$host) {
            return false;
        }
        foreach ((array)($this->config['coverproxyAllowedHosts'] ?? []) as $regEx) {
            if (preg_match($regEx, $host)) {
                return true;
            }
        }
        return false;
    }

        if (!empty($url) && $this->proxyAllowedForUrl($url)) {
        $config = $container->get(\VuFind\Config\PluginManager::class)->get('config');
        $configArray = $config?->Content?->toArray() ?? [];
            $container->get(\VuFind\Session\Settings::class),
            $configArray