CVE-2021-3758: Server-Side Request Forgery (SSRF) (CWE-918)
value: bookstack is vulnerable to Server-Side Request Forgery (SSRF)
CWE description: The web server receives a URL or similar request from an upstream component and retrieves the contents of this URL, but it does not sufficiently ensure that the request is being sent to the expected destination.
CVSS v3 base score: 6.5
Published: 2021-09-02T12:15Z
Repository: https://github.com/bookstackapp/bookstack
Language: PHP
Files changed: .env.example.complete, app/Config/app.php, app/Config/dompdf.php, app/Entities/Tools/ExportFormatter.php, phpunit.xml, tests/Entity/ExportTest.php, tests/Unit/ConfigTest.php
Commit date: 2021-08-31 20:22:42 +0100
Commit message: Added untrusted server fetching control

WKHTMLtoPDF provides limited control for external fetching
so that will now be disabled by default unless
ALLOW_UNTRUSTED_SERVER_FETCHING=true is specifically set.
This new option will also control DOMPDF fetching.

Diff: diff --git a/.env.example.complete b/.env.example.complete
index 26df8f3c..49d834ff 100644
--- a/.env.example.complete
+++ b/.env.example.complete
@@ -279,10 +279,16 @@ ALLOW_CONTENT_SCRIPTS=false
 # Can be 'true', 'false' or 'null'.
 # The behaviour of the default 'null' option will depend on the 'app-public' admin setting.
 # Contents of the robots.txt file can be overridden, making this option obsolete.
 ALLOW_ROBOTS=null
 
+# Allow server-side fetches to be performed to potentially unknown
+# and user-provided locations. Primarily used in exports when loading
+# in externally referenced assets.
+# Can be 'true' or 'false'.
+ALLOW_UNTRUSTED_SERVER_FETCHING=false
+
 # A list of hosts that BookStack can be iframed within.
 # Space separated if multiple. BookStack host domain is auto-inferred.
 # For Example: ALLOWED_IFRAME_HOSTS="https://example.com https://a.example.com"
 # Setting this option will also auto-adjust cookies to be SameSite=None.
 ALLOWED_IFRAME_HOSTS=null
diff --git a/app/Config/app.php b/app/Config/app.php
index a09c329c..6f190106 100755
--- a/app/Config/app.php
+++ b/app/Config/app.php
@@ -34,10 +34,15 @@ return [
     // Allow <script> tags to entered within page content.
     // <script> tags are escaped by default.
     // Even when overridden the WYSIWYG editor may still escape script content.
     'allow_content_scripts' => env('ALLOW_CONTENT_SCRIPTS', false),
 
+    # Allow server-side fetches to be performed to potentially unknown
+    # and user-provided locations. Primarily used in exports when loading
+    # in externally referenced assets.
+    'allow_untrusted_server_fetching' => env('ALLOW_UNTRUSTED_SERVER_FETCHING', false),
+
     // Override the default behaviour for allowing crawlers to crawl the instance.
     // May be ignored if view has be overridden or modified.
     // Defaults to null since, if not set, 'app-public' status used instead.
     'allow_robots' => env('ALLOW_ROBOTS', null),
 
diff --git a/app/Config/dompdf.php b/app/Config/dompdf.php
index 71ea716f..cf07312e 100644
--- a/app/Config/dompdf.php
+++ b/app/Config/dompdf.php
@@ -35,30 +35,30 @@ return [
          * Courier, Courier-Bold, Courier-BoldOblique, Courier-Oblique,
          * Helvetica, Helvetica-Bold, Helvetica-BoldOblique, Helvetica-Oblique,
          * Times-Roman, Times-Bold, Times-BoldItalic, Times-Italic,
          * Symbol, ZapfDingbats.
          */
-        'DOMPDF_FONT_DIR' => storage_path('fonts/'),  // advised by dompdf (https://github.com/dompdf/dompdf/pull/782)
+        'font_dir' => storage_path('fonts/'),  // advised by dompdf (https://github.com/dompdf/dompdf/pull/782)
 
         /**
          * The location of the DOMPDF font cache directory.
          *
          * This directory contains the cached font metrics for the fonts used by DOMPDF.
          * This directory can be the same as DOMPDF_FONT_DIR
          *
          * Note: This directory must exist and be writable by the webserver process.
          */
-        'DOMPDF_FONT_CACHE' => storage_path('fonts/'),
+        'font_cache' => storage_path('fonts/'),
 
         /**
          * The location of a temporary directory.
          *
          * The directory specified must be writeable by the webserver process.
          * The temporary directory is required to download remote images and when
          * using the PFDLib back end.
          */
-        'DOMPDF_TEMP_DIR' => sys_get_temp_dir(),
+        'temp_dir' => sys_get_temp_dir(),
 
         /**
          * ==== IMPORTANT ====.
          *
          * dompdf's "chroot": Prevents dompdf from accessing system files or other
@@ -68,27 +68,27 @@ return [
          * should be an absolute path.
          * This is only checked on command line call by dompdf.php, but not by
          * direct class use like:
          * $dompdf = new DOMPDF();  $dompdf->load_html($htmldata); $dompdf->render(); $pdfdata = $dompdf->output();
          */
-        'DOMPDF_CHROOT' => realpath(base_path()),
+        'chroot' => realpath(base_path()),
 
         /**
          * Whether to use Unicode fonts or not.
          *
          * When set to true the PDF backend must be set to "CPDF" and fonts must be
          * loaded via load_font.php.
          *
          * When enabled, dompdf can support all Unicode glyphs. Any glyphs used in a
          * document must be present in your fonts, however.
          */
-        'DOMPDF_UNICODE_ENABLED' => true,
+        'unicode_enabled' => true,
 
         /**
          * Whether to enable font subsetting or not.
          */
-        'DOMPDF_ENABLE_FONTSUBSETTING' => false,
+        'enable_fontsubsetting' => false,
 
         /**
          * The PDF rendering backend to use.
          *
          * Valid settings are 'PDFLib', 'CPDF' (the bundled R&OS PDF class), 'GD' and
@@ -113,11 +113,11 @@ return [
          *
          * @link http://www.pdflib.com
          * @link http://www.ros.co.nz/pdf
          * @link http://www.php.net/image
          */
-        'DOMPDF_PDF_BACKEND' => 'CPDF',
+        'pdf_backend' => 'CPDF',
 
         /**
          * PDFlib license key.
          *
          * If you are using a licensed, commercial version of PDFlib, specify
@@ -139,29 +139,29 @@ return [
          * Note: aural is deprecated in CSS 2.1 because it is replaced by speech in CSS 3.
          * Note, even though the generated pdf file is intended for print output,
          * the desired content might be different (e.g. screen or projection view of html file).
          * Therefore allow specification of content here.
          */
-        'DOMPDF_DEFAULT_MEDIA_TYPE' => 'print',
+        'default_media_type' => 'print',
 
         /**
          * The default paper size.
          *
          * North America standard is "letter"; other countries generally "a4"
          *
          * @see CPDF_Adapter::PAPER_SIZES for valid sizes ('letter', 'legal', 'A4', etc.)
          */
-        'DOMPDF_DEFAULT_PAPER_SIZE' => 'a4',
+        'default_paper_size' => 'a4',
 
         /**
          * The default font family.
          *
          * Used if no suitable fonts can be found. This must exist in the font folder.
          *
          * @var string
          */
-        'DOMPDF_DEFAULT_FONT' => 'dejavu sans',
+        'default_font' => 'dejavu sans',
 
         /**
          * Image DPI setting.
          *
          * This setting determines the default DPI setting for images and fonts.  The
@@ -192,11 +192,11 @@ return [
          * In images, <img> size in pixel attribute, img css style, are overriding
          * the real image dimension in px for rendering.
          *
          * @var int
          */
-        'DOMPDF_DPI' => 96,
+        'dpi' => 96,
 
         /**
          * Enable inline PHP.
          *
          * If this setting is set to true then DOMPDF will automatically evaluate
@@ -206,21 +206,21 @@ return [
          * pages) is a security risk.  Set this option to false if you wish to process
          * untrusted documents.
          *
          * @var bool
          */
-        'DOMPDF_ENABLE_PHP' => false,
+        'enable_php' => false,
 
         /**
          * Enable inline Javascript.
          *
          * If this setting is set to true then DOMPDF will automatically insert
          * JavaScript code contained within <script type="text/javascript"> ... </script> tags.
          *
          * @var bool
          */
-        'DOMPDF_ENABLE_JAVASCRIPT' => false,
+        'enable_javascript' => false,
 
         /**
          * Enable remote file access.
          *
          * If this setting is set to true, DOMPDF will access remote sites for
@@ -235,29 +235,29 @@ return [
          * tracing back appears to being downloaded by your server, or allows malicious php code
          * in remote html pages to be executed by your server with your account privileges.
          *
          * @var bool
          */
-        'DOMPDF_ENABLE_REMOTE' => true,
+        'enable_remote' => env('ALLOW_UNTRUSTED_SERVER_FETCHING', false),
 
         /**
          * A ratio applied to the fonts height to be more like browsers' line height.
          */
-        'DOMPDF_FONT_HEIGHT_RATIO' => 1.1,
+        'font_height_ratio' => 1.1,
 
         /**
          * Enable CSS float.
          *
          * Allows people to disabled CSS float support
          *
          * @var bool
          */
-        'DOMPDF_ENABLE_CSS_FLOAT' => true,
+        'enable_css_float' => true,
 
         /**
          * Use the more-than-experimental HTML5 Lib parser.
          */
-        'DOMPDF_ENABLE_HTML5PARSER' => true,
+        'enable_html5parser' => true,
 
     ],
 
 ];
diff --git a/app/Entities/Tools/ExportFormatter.php b/app/Entities/Tools/ExportFormatter.php
index c299f9c7..05d0ff13 100644
--- a/app/Entities/Tools/ExportFormatter.php
+++ b/app/Entities/Tools/ExportFormatter.php
@@ -138,11 +138,11 @@ class ExportFormatter
      * @throws Exception
      */
     protected function htmlToPdf(string $html): string
     {
         $containedHtml = $this->containHtml($html);
-        $useWKHTML = config('snappy.pdf.binary') !== false;
+        $useWKHTML = config('snappy.pdf.binary') !== false && config('app.allow_untrusted_server_fetching') === true;
         if ($useWKHTML) {
             $pdf = SnappyPDF::loadHTML($containedHtml);
             $pdf->setOption('print-media-type', true);
         } else {
             $pdf = DomPDF::loadHTML($containedHtml);
diff --git a/phpunit.xml b/phpunit.xml
index 75c89ec3..7e0da05d 100644
--- a/phpunit.xml
+++ b/phpunit.xml
@@ -35,10 +35,11 @@
     <server name="BCRYPT_ROUNDS" value="4"/>
     <server name="MAIL_DRIVER" value="array"/>
     <server name="LOG_CHANNEL" value="single"/>
     <server name="AUTH_METHOD" value="standard"/>
     <server name="DISABLE_EXTERNAL_SERVICES" value="true"/>
+    <server name="ALLOW_UNTRUSTED_SERVER_FETCHING" value="false"/>
     <server name="AVATAR_URL" value=""/>
     <server name="LDAP_START_TLS" value="false"/>
     <server name="LDAP_VERSION" value="3"/>
     <server name="SESSION_SECURE_COOKIE" value="null"/>
     <server name="STORAGE_TYPE" value="local"/>
diff --git a/tests/Entity/ExportTest.php b/tests/Entity/ExportTest.php
index 7031c387..aebc5f24 100644
--- a/tests/Entity/ExportTest.php
+++ b/tests/Entity/ExportTest.php
@@ -364,6 +364,22 @@ class ExportTest extends TestCase
             $resp->assertDontSee('/export/pdf');
             $resp = $this->get($entity->getUrl('/export/pdf'));
             $this->assertPermissionError($resp);
         }
     }
+
+    public function test_wkhtmltopdf_only_used_when_allow_untrusted_is_true()
+    {
+        /** @var Page $page */
+        $page = Page::query()->first();
+
+        config()->set('snappy.pdf.binary', '/abc123');
+        config()->set('app.allow_untrusted_server_fetching', false);
+
+        $resp = $this->asEditor()->get($page->getUrl('/export/pdf'));
+        $resp->assertStatus(200); // Sucessful response with invalid snappy binary indicates dompdf usage.
+
+        config()->set('app.allow_untrusted_server_fetching', true);
+        $resp = $this->get($page->getUrl('/export/pdf'));
+        $resp->assertStatus(500); // Bad response indicates wkhtml usage
+    }
 }
diff --git a/tests/Unit/ConfigTest.php b/tests/Unit/ConfigTest.php
index f45d2013..207fb7f5 100644
--- a/tests/Unit/ConfigTest.php
+++ b/tests/Unit/ConfigTest.php
@@ -74,10 +74,16 @@ class ConfigTest extends TestCase
             'saml2.onelogin.security.requestedAuthnContext',
             ['urn:federation:authentication:windows', 'urn:federation:authentication:linux']
         );
     }
 
+    public function test_dompdf_remote_fetching_controlled_by_allow_untrusted_server_fetching_false()
+    {
+        $this->checkEnvConfigResult('ALLOW_UNTRUSTED_SERVER_FETCHING', 'false', 'dompdf.defines.enable_remote', false);
+        $this->checkEnvConfigResult('ALLOW_UNTRUSTED_SERVER_FETCHING', 'true', 'dompdf.defines.enable_remote', true);
+    }
+
     /**
      * Set an environment variable of the given name and value
      * then check the given config key to see if it matches the given result.
      * Providing a null $envVal clears the variable.
      *

Vulnerable code:         'DOMPDF_FONT_DIR' => storage_path('fonts/'),  // advised by dompdf (https://github.com/dompdf/dompdf/pull/782)
        'DOMPDF_FONT_CACHE' => storage_path('fonts/'),
        'DOMPDF_TEMP_DIR' => sys_get_temp_dir(),
        'DOMPDF_CHROOT' => realpath(base_path()),
        'DOMPDF_UNICODE_ENABLED' => true,
        'DOMPDF_ENABLE_FONTSUBSETTING' => false,
        'DOMPDF_PDF_BACKEND' => 'CPDF',
        'DOMPDF_DEFAULT_MEDIA_TYPE' => 'print',
        'DOMPDF_DEFAULT_PAPER_SIZE' => 'a4',
        'DOMPDF_DEFAULT_FONT' => 'dejavu sans',
        'DOMPDF_DPI' => 96,
        'DOMPDF_ENABLE_PHP' => false,
        'DOMPDF_ENABLE_JAVASCRIPT' => false,
        'DOMPDF_ENABLE_REMOTE' => true,
        'DOMPDF_FONT_HEIGHT_RATIO' => 1.1,
        'DOMPDF_ENABLE_CSS_FLOAT' => true,
        'DOMPDF_ENABLE_HTML5PARSER' => true,
        $useWKHTML = config('snappy.pdf.binary') !== false;
Fixed code: # Allow server-side fetches to be performed to potentially unknown
# and user-provided locations. Primarily used in exports when loading
# in externally referenced assets.
# Can be 'true' or 'false'.
ALLOW_UNTRUSTED_SERVER_FETCHING=false

    # Allow server-side fetches to be performed to potentially unknown
    # and user-provided locations. Primarily used in exports when loading
    # in externally referenced assets.
    'allow_untrusted_server_fetching' => env('ALLOW_UNTRUSTED_SERVER_FETCHING', false),

        'font_dir' => storage_path('fonts/'),  // advised by dompdf (https://github.com/dompdf/dompdf/pull/782)
        'font_cache' => storage_path('fonts/'),
        'temp_dir' => sys_get_temp_dir(),
        'chroot' => realpath(base_path()),
        'unicode_enabled' => true,
        'enable_fontsubsetting' => false,
        'pdf_backend' => 'CPDF',
        'default_media_type' => 'print',
        'default_paper_size' => 'a4',
        'default_font' => 'dejavu sans',
        'dpi' => 96,
        'enable_php' => false,
        'enable_javascript' => false,
        'enable_remote' => env('ALLOW_UNTRUSTED_SERVER_FETCHING', false),
        'font_height_ratio' => 1.1,
        'enable_css_float' => true,
        'enable_html5parser' => true,
        $useWKHTML = config('snappy.pdf.binary') !== false && config('app.allow_untrusted_server_fetching') === true;
    <server name="ALLOW_UNTRUSTED_SERVER_FETCHING" value="false"/>

    public function test_wkhtmltopdf_only_used_when_allow_untrusted_is_true()
    {
        /** @var Page $page */
        $page = Page::query()->first();

        config()->set('snappy.pdf.binary', '/abc123');
        config()->set('app.allow_untrusted_server_fetching', false);

        $resp = $this->asEditor()->get($page->getUrl('/export/pdf'));
        $resp->assertStatus(200); // Sucessful response with invalid snappy binary indicates dompdf usage.

        config()->set('app.allow_untrusted_server_fetching', true);
        $resp = $this->get($page->getUrl('/export/pdf'));
        $resp->assertStatus(500); // Bad response indicates wkhtml usage
    }
    public function test_dompdf_remote_fetching_controlled_by_allow_untrusted_server_fetching_false()
    {
        $this->checkEnvConfigResult('ALLOW_UNTRUSTED_SERVER_FETCHING', 'false', 'dompdf.defines.enable_remote', false);
        $this->checkEnvConfigResult('ALLOW_UNTRUSTED_SERVER_FETCHING', 'true', 'dompdf.defines.enable_remote', true);
    }