diff --git a/src/Wallabag/CoreBundle/Controller/ExportController.php b/src/Wallabag/CoreBundle/Controller/ExportController.php index 2db335527c..0599c401d7 100644 --- 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 index 41b995efaa..9d983200ca 100644 --- 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');