Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge pull request from GHSA-qwx8-mxxx-mg96
ExportController: fix improper authorization vulnerability
  • Loading branch information
j0k3r committed Feb 1, 2023
2 parents 9e9aede + 0fdd9aa commit 0f7460d
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 4 deletions.
17 changes: 14 additions & 3 deletions src/Wallabag/CoreBundle/Controller/ExportController.php
Expand Up @@ -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.
Expand All @@ -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')
Expand Down
14 changes: 13 additions & 1 deletion tests/Wallabag/CoreBundle/Controller/ExportControllerTest.php
Expand Up @@ -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();
Expand All @@ -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');
Expand Down

0 comments on commit 0f7460d

Please sign in to comment.