[TikiWiki-commits] [Git][tikiwiki/tiki][30.x] [FIX] MediaWiki importer: block non-http(s) attachment URLs and sanitize filenames

Alfred Syatsukwa pushed to branch 30.x at Tiki Wiki CMS Groupware / Tiki


Commits:
f32b2f7b by Alfred Syatsukwa at 2026-08-18T12:11:16+00:00
[FIX] MediaWiki importer: block non-http(s) attachment URLs and sanitize filenames
---
* [FIX] MediaWiki importer: block non-http(s) attachment URLs and sanitize filenames
---
* [FIX] MediaWiki importer: block non-http(s) attachment URLs and sanitize filenames

(cherry picked from commit 775f3e8053827faa05864c79943887d1c69ae129)

See merge request tikiwiki/tiki!10947

(cherry picked from commit dffdb584f7f51d1181ad567f37840abbd62b82be)

See merge request tikiwiki/tiki!10949

- - - - -


4 changed files:

- lib/Importer/WikiMediawiki.php
- lib/test/Importer/WikiMediawikiTest.php
- lib/test/Importer/fixtures/mediawiki_invalid_upload.xml
- lib/test/Importer/fixtures/mediawiki_sample.xml


Changes:

=====================================
lib/Importer/WikiMediawiki.php
=====================================
@@ -329,9 +329,14 @@ class WikiMediawiki extends Wiki
                 $i = $attachments->length - 1;
                 $lastVersion = $attachments->item($i);
 
-                $fileName = $lastVersion->getElementsByTagName('filename')->item(0)->nodeValue;
+                $fileName = basename($lastVersion->getElementsByTagName('filename')->item(0)->nodeValue);
                 $fileUrl = $lastVersion->getElementsByTagName('src')->item(0)->nodeValue;
 
+                if ($fileName === '' || $fileName === '.' || $fileName === '..') {
+                    $this->saveAndDisplayLog(tr('File not imported: invalid attachment filename.') . "\n", true);
+                    continue;
+                }
+
                 if (file_exists($this->attachmentsDestDir . $fileName)) {
                     $this->saveAndDisplayLog(
                         tr(
@@ -344,20 +349,21 @@ class WikiMediawiki extends Wiki
                     continue;
                 }
 
-                // Prevent SSRF: attachment URLs come from the imported XML dump
-                // which may be untrusted. Block private/reserved IP targets.
-                // Only check URLs with a scheme (http/https); relative file paths
-                // are local references and not a network SSRF vector.
-                if (preg_match('#^https?://#i', $fileUrl)) {
-                    $ssrf = \Tiki\Security\SsrfLib::fromPrefs();
-                    if (! $ssrf->isUrlAllowed($fileUrl)) {
-                        $this->saveAndDisplayLog(tr('File %0 not imported: URL targets a private or reserved address.', $fileName) . "\n", true);
-                        continue;
-                    }
+                // Prevent SSRF / local-file disclosure: attachment <src> values come
+                // from the imported XML dump and may be untrusted. Always validate
+                // with SsrfLib (http/https allowlist + reject private/reserved hosts)
+                // so file://, php://, and other stream wrappers cannot bypass the gate.
+                $ssrf = \Tiki\Security\SsrfLib::fromPrefs();
+                if (! $ssrf->isUrlAllowed($fileUrl)) {
+                    $this->saveAndDisplayLog(
+                        tr('File %0 not imported: attachment URL must be a public http(s) address.', $fileName) . "\n",
+                        true
+                    );
+                    continue;
                 }
 
-                if (@fopen($fileUrl, 'r')) {
-                    $attachmentContent = @file_get_contents($fileUrl);
+                $attachmentContent = $this->fetchAttachmentContents($fileUrl);
+                if ($attachmentContent !== false) {
                     $newFile = fopen($this->attachmentsDestDir . $fileName, 'w');
                     fwrite($newFile, $attachmentContent);
                     $this->saveAndDisplayLog(tr('File %0 successfully imported!', $fileName) . "\n");
@@ -368,6 +374,18 @@ class WikiMediawiki extends Wiki
         }
     }
 
+    /**
+     * Fetch attachment bytes from a validated URL.
+     * Isolated for unit tests so downloads can be mocked without network I/O.
+     *
+     * @param string $fileUrl
+     * @return string|false
+     */
+    protected function fetchAttachmentContents($fileUrl)
+    {
+        return @file_get_contents($fileUrl);
+    }
+
     /**
      * Parse an DOM representation of a Mediawiki page and return all the values
      * that will be imported (page name, page content for all revisions). The


=====================================
lib/test/Importer/WikiMediawikiTest.php
=====================================
@@ -200,32 +200,22 @@ class WikiMediawikiTest extends AbstractImporterTestCase
     {
         ob_start();
 
-        $this->obj->attachmentsDestDir = __DIR__ . '/fixtures/';
-
-        $sourceAttachments = ['sourceTest.jpg', 'sourceTest2.jpg'];
-        $destAttachments = ['test.jpg', 'test2.jpg'];
-        $i = count($sourceAttachments) - 1;
-        $cwd = getcwd();
-        chdir(__DIR__);
-
-        while ($i >= 0) {
-            fopen($this->obj->attachmentsDestDir . $sourceAttachments[$i], 'w');
-            $i--;
-        }
+        $obj = $this->getMockBuilder(WikiMediawiki::class)
+            ->onlyMethods(['fetchAttachmentContents'])
+            ->getMock();
+        $obj->attachmentsDestDir = __DIR__ . '/fixtures/';
+        $obj->method('fetchAttachmentContents')->willReturn('fake-image-bytes');
 
-        $this->obj->dom = new DOMDocument();
-        $this->obj->dom->load(__DIR__ . '/fixtures/mediawiki_sample.xml');
-        $this->obj->downloadAttachments();
+        $obj->dom = new DOMDocument();
+        $obj->dom->load(__DIR__ . '/fixtures/mediawiki_sample.xml');
+        $obj->downloadAttachments();
 
-        $i = count($sourceAttachments) - 1;
-        while ($i >= 0) {
-            $filePath = $this->obj->attachmentsDestDir . $destAttachments[$i];
+        foreach (['test.jpg', 'test2.jpg'] as $attachment) {
+            $filePath = $obj->attachmentsDestDir . $attachment;
             $this->assertFileExists($filePath);
+            $this->assertSame('fake-image-bytes', file_get_contents($filePath));
             unlink($filePath);
-            unlink($this->obj->attachmentsDestDir . $sourceAttachments[$i]);
-            $i--;
         }
-        chdir($cwd);
 
         $output = ob_get_clean();
         $this->assertEquals("\n\nImporting attachments:\nFile test2.jpg successfully imported!\nFile test.jpg successfully imported!\n", $output);
@@ -271,13 +261,57 @@ class WikiMediawikiTest extends AbstractImporterTestCase
     {
         ob_start();
 
+        $obj = $this->getMockBuilder(WikiMediawiki::class)
+            ->onlyMethods(['fetchAttachmentContents'])
+            ->getMock();
+        $obj->attachmentsDestDir = __DIR__ . '/fixtures/';
+        $obj->method('fetchAttachmentContents')->willReturn(false);
+        $obj->dom = new DOMDocument();
+        $obj->dom->load(__DIR__ . '/fixtures/mediawiki_invalid_upload.xml');
+        $obj->downloadAttachments();
+
+        $output = ob_get_clean();
+        $this->assertEquals("\n\nImporting attachments:\nUnable to download file Qlandkartegt-0.11.1.tar.gz. File not found.\nUnable to download file Passelivre.jpg. File not found.\n", $output);
+    }
+
+    public function testDownloadAttachmentsShouldRejectNonHttpSrc(): void
+    {
+        ob_start();
+
+        $xml = <<<'XML'
+<mediawiki>
+  <page>
+    <upload>
+      <filename>evil.png</filename>
+      <src>file:///etc/passwd</src>
+      <size>1</size>
+    </upload>
+  </page>
+  <page>
+    <upload>
+      <filename>local.php.png</filename>
+      <src>db/local.php</src>
+      <size>1</size>
+    </upload>
+  </page>
+</mediawiki>
+XML;
+
         $this->obj->attachmentsDestDir = __DIR__ . '/fixtures/';
         $this->obj->dom = new DOMDocument();
-        $this->obj->dom->load(__DIR__ . '/fixtures/mediawiki_invalid_upload.xml');
+        $this->obj->dom->loadXML($xml);
         $this->obj->downloadAttachments();
 
+        $this->assertFileDoesNotExist($this->obj->attachmentsDestDir . 'evil.png');
+        $this->assertFileDoesNotExist($this->obj->attachmentsDestDir . 'local.php.png');
+
         $output = ob_get_clean();
-        $this->assertEquals("\n\nImporting attachments:\nUnable to download file Qlandkartegt-0.11.1.tar.gz. File not found.\nUnable to download file Passelivre.jpg. File not found.\n", $output);
+        $this->assertEquals(
+            "\n\nImporting attachments:\n"
+            . "File evil.png not imported: attachment URL must be a public http(s) address.\n"
+            . "File local.php.png not imported: attachment URL must be a public http(s) address.\n",
+            $output
+        );
     }
 
     public function testExtractInfo(): void


=====================================
lib/test/Importer/fixtures/mediawiki_invalid_upload.xml
=====================================
@@ -8,7 +8,7 @@
       </contributor>
       <comment>adsfasdf</comment>
       <filename>Qlandkartegt-0.11.1.tar.gz</filename>
-      <src>fixtures/test3.jpg</src>
+      <src>https://example.com/missing-qlandkartegt.tar.gz</src>
       <size>3252598</size>
     </upload>
     <upload>
@@ -19,7 +19,7 @@
       </contributor>
       <comment>adsfasdf</comment>
       <filename>Qlandkartegt-0.11.1.tar.gz</filename>
-      <src>fixtures/test3.jpg</src>
+      <src>https://example.com/missing-qlandkartegt.tar.gz</src>
       <size>3252598</size>
     </upload>
   </page>
@@ -32,7 +32,7 @@
       </contributor>
       <comment />
       <filename>Passelivre.jpg</filename>
-      <src>fixtures/test3.jpg</src>
+      <src>https://example.com/missing-passelivre.jpg</src>
       <size>94751</size>
     </upload>
   </page>


=====================================
lib/test/Importer/fixtures/mediawiki_sample.xml
=====================================
@@ -190,7 +190,7 @@
       </contributor>
       <comment>adsfasdf</comment>
       <filename>test3.jpg</filename>
-      <src>fixtures/sourceTest3.jpg</src>
+      <src>https://example.com/sourceTest3.jpg</src>
       <size>3252598</size>
     </upload>
     <upload>
@@ -201,7 +201,7 @@
       </contributor>
       <comment>adsfasdf</comment>
       <filename>test2.jpg</filename>
-      <src>fixtures/sourceTest2.jpg</src>
+      <src>https://example.com/sourceTest2.jpg</src>
       <size>3252598</size>
     </upload>
   </page>
@@ -234,7 +234,7 @@
       </contributor>
       <comment />
       <filename>test.jpg</filename>
-      <src>fixtures/sourceTest.jpg</src>
+      <src>https://example.com/sourceTest.jpg</src>
       <size>94751</size>
     </upload>
   </page>



View it on GitLab: https://gitlab.com/tikiwiki/tiki/-/commit/f32b2f7b61530b5a26847ecd96f4167fbc7fafb3

-- 
View it on GitLab: https://gitlab.com/tikiwiki/tiki/-/commit/f32b2f7b61530b5a26847ecd96f4167fbc7fafb3
You're receiving this email because of your account on gitlab.com. Manage all notifications: https://gitlab.com/-/profile/notifications | Help: https://gitlab.com/help

_______________________________________________
TikiWiki-cvs mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/tikiwiki-cvs