Merge pull request from GHSA-qwx8-mxxx-mg96

ExportController: fix improper authorization vulnerability
wallabag · Feb 1, 2023 · 0f7460d · 0f7460d
2 parents 9e9aede + 0fdd9aa
commit 0f7460d
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 4 deletions.
diff --git a/src/Wallabag/CoreBundle/Controller/ExportController.php b/src/Wallabag/CoreBundle/Controller/ExportController.php
@@ -6,7 +6,6 @@
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
 use Symfony\Component\Routing\Annotation\Route;
-use Wallabag\CoreBundle\Entity\Entry;
 
 /**
  * The try/catch can be removed once all formats will be implemented.
@@ -26,9 +25,21 @@ class ExportController extends Controller
      *
      * @return \Symfony\Component\HttpFoundation\Response
      */
-    public function downloadEntryAction(Entry $entry, $format)
+    public function downloadEntryAction(Request $request, $format)
     {
-        try {
+         try {
+            $entry = $this->get('wallabag_core.entry_repository')
+                ->find((int) $request->query->get('id'));
+
+            /**
+             * We duplicate EntryController::checkUserAction here as a quick fix for an improper authorization vulnerability
+             *
+             * This should be eventually rewritten
+            */
+            if (null === $entry || null === $this->getUser() || $this->getUser()->getId() !== $entry->getUser()->getId()) {
+                throw new NotFoundHttpException();
+            }
+
             return $this->get('wallabag_core.helper.entries_export')
                 ->setEntries($entry)
                 ->updateTitle('entry')

diff --git a/tests/Wallabag/CoreBundle/Controller/ExportControllerTest.php b/tests/Wallabag/CoreBundle/Controller/ExportControllerTest.php
@@ -57,7 +57,7 @@ public function testUnsupportedFormatExport()
         $this->assertSame(404, $client->getResponse()->getStatusCode());
     }
 
-    public function testBadEntryId()
+    public function testNonExistingEntryId()
     {
         $this->logInAs('admin');
         $client = $this->getClient();
@@ -67,6 +67,18 @@ public function testBadEntryId()
         $this->assertSame(404, $client->getResponse()->getStatusCode());
     }
 
+    public function testForbiddenEntryId()
+    {
+        $this->logInAs('admin');
+        $client = $this->getClient();
+
+        // Entry with id 3 is owned by the user bob
+        // See EntryFixtures
+        $client->request('GET', '/export/3.mobi');
+
+        $this->assertSame(404, $client->getResponse()->getStatusCode());
+    }
+
     public function testEpubExport()
     {
         $this->logInAs('admin');