Merge pull request #6267 from wallabag/release/2.5.3

Prepare 2.5.3

CHANGELOG.md

@@ -1,5 +1,15 @@
 # Changelog
 
+## [2.5.3](https://github.com/wallabag/wallabag/tree/2.5.3)
+   [Full Changelog](https://github.com/wallabag/wallabag/compare/2.5.2...2.5.3)
+
+### Security fixes
+* Fix GHSA-qwx8-mxxx-mg96 https://github.com/wallabag/wallabag/commit/0f7460dbab9e29f4f7d2944aca20210f828b6abb by @Kdecherf, thanks to @bAuh0lz
+* Fix GHSA-mrqx-mjc4-vfh3 https://github.com/wallabag/wallabag/commit/5ac6b6bff9e2e3a87fd88c2904ff3c6aac40722e by @Kdecherf, thanks to @bAuh0lz
+
+### Meta
+* Update deps before 2.5.3 by @j0k3r in https://github.com/wallabag/wallabag/pull/6241
+
 ## [2.5.2](https://github.com/wallabag/wallabag/tree/2.5.2)
    [Full Changelog](https://github.com/wallabag/wallabag/compare/2.5.1...2.5.2)
 

app/config/wallabag.yml

@@ -1,5 +1,5 @@
 wallabag_core:
-    version: 2.5.2
+    version: 2.5.3
     paypal_url: "https://www.paypal.com/cgi-bin/webscr?cmd=_s-xclick&hosted_button_id=9UBA65LG3FX9Y&lc=gb"
     languages:
         en: 'English'

package.json

@@ -36,34 +36,34 @@
     "url": "https://github.com/wallabag/wallabag/issues"
   },
   "devDependencies": {
-    "@babel/core": "^7.19.3",
+    "@babel/core": "^7.20.12",
     "@babel/eslint-parser": "^7.19.1",
-    "@babel/preset-env": "^7.19.4",
-    "autoprefixer": "^10.4.12",
-    "babel-loader": "^8.2.5",
-    "css-loader": "^6.7.1",
-    "eslint": "^8.25.0",
+    "@babel/preset-env": "^7.20.2",
+    "autoprefixer": "^10.4.13",
+    "babel-loader": "^9.1.2",
+    "css-loader": "^6.7.3",
+    "eslint": "^8.32.0",
     "eslint-config-airbnb-base": "^15.0.0",
-    "eslint-plugin-import": "^2.26.0",
+    "eslint-plugin-import": "^2.27.4",
     "eslint-webpack-plugin": "^3.2.0",
     "file-loader": "^6.2.0",
     "lato-font": "^3.0.0",
-    "mini-css-extract-plugin": "^2.6.1",
-    "node-sass": "^7.0.3",
-    "postcss": "^8.4.18",
-    "postcss-loader": "^7.0.1",
-    "postcss-scss": "^4.0.5",
-    "sass": "^1.55.0",
-    "sass-loader": "^13.1.0",
+    "mini-css-extract-plugin": "^2.7.2",
+    "node-sass": "^8.0.0",
+    "postcss": "^8.4.21",
+    "postcss-loader": "^7.0.2",
+    "postcss-scss": "^4.0.6",
+    "sass": "^1.57.1",
+    "sass-loader": "^13.2.0",
     "style-loader": "^3.3.1",
-    "stylelint": "^14.14.0",
-    "stylelint-config-standard": "^28.0.0",
+    "stylelint": "^14.16.1",
+    "stylelint-config-standard": "^29.0.0",
     "stylelint-scss": "^4.3.0",
     "stylelint-webpack-plugin": "^3.3.0",
     "terser-webpack-plugin": "^5.3.6",
     "url-loader": "^4.1.1",
-    "webpack": "^5.74.0",
-    "webpack-cli": "^4.10.0",
+    "webpack": "^5.75.0",
+    "webpack-cli": "^5.0.1",
     "webpack-dev-server": "^4.11.1",
     "webpack-manifest-plugin": "^5.0.0",
     "webpack-merge": "^5.7.3"
@@ -72,9 +72,9 @@
     "annotator": "wallabag/annotator#master",
     "clipboard": "^2.0.11",
     "hammerjs": "^2.0.8",
-    "highlight.js": "^11.6.0",
+    "highlight.js": "^11.7.0",
     "icomoon-free-npm": "^0.0.0",
-    "jquery": "^3.6.1",
+    "jquery": "^3.6.3",
     "jquery.cookie": "^1.4.1",
     "jr-qrcode": "^1.0.7",
     "material-design-icons-iconfont": "^6.7.0",

src/Wallabag/AnnotationBundle/Controller/WallabagAnnotationController.php

@@ -3,9 +3,9 @@
 namespace Wallabag\AnnotationBundle\Controller;
 
 use FOS\RestBundle\Controller\FOSRestController;
-use Sensio\Bundle\FrameworkExtraBundle\Configuration\ParamConverter;
 use Symfony\Component\HttpFoundation\JsonResponse;
 use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
 use Wallabag\AnnotationBundle\Entity\Annotation;
 use Wallabag\AnnotationBundle\Form\EditAnnotationType;
 use Wallabag\AnnotationBundle\Form\NewAnnotationType;
@@ -25,7 +25,7 @@ class WallabagAnnotationController extends FOSRestController
         $annotationRows = $this
             ->getDoctrine()
             ->getRepository('WallabagAnnotationBundle:Annotation')
-            ->findAnnotationsByPageId($entry->getId(), $this->getUser()->getId());
+            ->findByEntryIdAndUserId($entry->getId(), $this->getUser()->getId());
         $total = \count($annotationRows);
         $annotations = ['total' => $total, 'rows' => $annotationRows];
 
@@ -72,31 +72,35 @@ class WallabagAnnotationController extends FOSRestController
      *
      * @see Wallabag\ApiBundle\Controller\WallabagRestController
      *
-     * @ParamConverter("annotation", class="WallabagAnnotationBundle:Annotation")
-     *
      * @return JsonResponse
      */
-    public function putAnnotationAction(Annotation $annotation, Request $request)
+    public function putAnnotationAction(Request $request, int $annotation)
     {
-        $data = json_decode($request->getContent(), true);
+        try {
+            $annotation = $this->validateAnnotation($annotation, $this->getUser()->getId());
 
-        $form = $this->get('form.factory')->createNamed('', EditAnnotationType::class, $annotation, [
-            'csrf_protection' => false,
-            'allow_extra_fields' => true,
-        ]);
-        $form->submit($data);
+            $data = json_decode($request->getContent(), true, 512, \JSON_THROW_ON_ERROR);
 
-        if ($form->isValid()) {
-            $em = $this->getDoctrine()->getManager();
-            $em->persist($annotation);
-            $em->flush();
+            $form = $this->get('form.factory')->createNamed('', EditAnnotationType::class, $annotation, [
+                'csrf_protection' => false,
+                'allow_extra_fields' => true,
+            ]);
+            $form->submit($data);
 
-            $json = $this->get('jms_serializer')->serialize($annotation, 'json');
+            if ($form->isValid()) {
+                $em = $this->getDoctrine()->getManager();
+                $em->persist($annotation);
+                $em->flush();
 
-            return JsonResponse::fromJsonString($json);
+                $json = $this->get('jms_serializer')->serialize($annotation, 'json');
+
+                return JsonResponse::fromJsonString($json);
+            }
+
+            return $form;
+        } catch (\InvalidArgumentException $e) {
+            throw new NotFoundHttpException($e);
         }
-
-        return $form;
     }
 
     /**
@@ -104,18 +108,35 @@ class WallabagAnnotationController extends FOSRestController
      *
      * @see Wallabag\ApiBundle\Controller\WallabagRestController
      *
-     * @ParamConverter("annotation", class="WallabagAnnotationBundle:Annotation")
-     *
      * @return JsonResponse
      */
-    public function deleteAnnotationAction(Annotation $annotation)
+    public function deleteAnnotationAction(int $annotation)
+    {
+        try {
+            $annotation = $this->validateAnnotation($annotation, $this->getUser()->getId());
+
+            $em = $this->getDoctrine()->getManager();
+            $em->remove($annotation);
+            $em->flush();
+
+            $json = $this->get('jms_serializer')->serialize($annotation, 'json');
+
+            return (new JsonResponse())->setJson($json);
+        } catch (\InvalidArgumentException $e) {
+            throw new NotFoundHttpException($e);
+        }
+    }
+
+    private function validateAnnotation(int $annotationId, int $userId)
     {
         $em = $this->getDoctrine()->getManager();
-        $em->remove($annotation);
-        $em->flush();
 
-        $json = $this->get('jms_serializer')->serialize($annotation, 'json');
+        $annotation = $em->getRepository('WallabagAnnotationBundle:Annotation')->findOneByIdAndUserId($annotationId, $userId);
 
-        return (new JsonResponse())->setJson($json);
+        if (null === $annotation) {
+            throw new NotFoundHttpException();
+        }
+
+        return $annotation;
     }
 }

src/Wallabag/AnnotationBundle/DataFixtures/AnnotationFixtures.php

@@ -34,6 +34,15 @@ class AnnotationFixtures extends Fixture implements DependentFixtureInterface
 
         $this->addReference('annotation2', $annotation2);
 
+        $annotation3 = new Annotation($this->getReference('bob-user'));
+        $annotation3->setEntry($this->getReference('entry3'));
+        $annotation3->setText('This is my first annotation !');
+        $annotation3->setQuote('content');
+
+        $manager->persist($annotation3);
+
+        $this->addReference('annotation3', $annotation3);
+
         $manager->flush();
     }
 

src/Wallabag/AnnotationBundle/Repository/AnnotationRepository.php

@@ -41,6 +41,24 @@ class AnnotationRepository extends EntityRepository
         ;
     }
 
+    /**
+     * Find annotation by id and user.
+     *
+     * @param int $annotationId
+     * @param int $userId
+     *
+     * @return Annotation
+     */
+    public function findOneByIdAndUserId($annotationId, $userId)
+    {
+        return $this->createQueryBuilder('a')
+                    ->where('a.id = :annotationId')->setParameter('annotationId', $annotationId)
+                    ->andWhere('a.user = :userId')->setParameter('userId', $userId)
+                    ->setMaxResults(1)
+                    ->getQuery()
+                    ->getOneOrNullResult();
+    }
+
     /**
      * Find annotations for entry id.
      *
@@ -49,7 +67,7 @@ class AnnotationRepository extends EntityRepository
      *
      * @return array
      */
-    public function findAnnotationsByPageId($entryId, $userId)
+    public function findByEntryIdAndUserId($entryId, $userId)
     {
         return $this->createQueryBuilder('a')
             ->where('a.entry = :entryId')->setParameter('entryId', $entryId)
@@ -66,7 +84,7 @@ class AnnotationRepository extends EntityRepository
      *
      * @return array
      */
-    public function findLastAnnotationByPageId($entryId, $userId)
+    public function findLastAnnotationByUserId($entryId, $userId)
     {
         return $this->createQueryBuilder('a')
             ->where('a.entry = :entryId')->setParameter('entryId', $entryId)

src/Wallabag/ApiBundle/Controller/AnnotationRestController.php

@@ -3,7 +3,6 @@
 namespace Wallabag\ApiBundle\Controller;
 
 use Nelmio\ApiDocBundle\Annotation\ApiDoc;
-use Sensio\Bundle\FrameworkExtraBundle\Configuration\ParamConverter;
 use Symfony\Component\HttpFoundation\JsonResponse;
 use Symfony\Component\HttpFoundation\Request;
 use Wallabag\AnnotationBundle\Entity\Annotation;
@@ -63,11 +62,9 @@ class AnnotationRestController extends WallabagRestController
      *      }
      * )
      *
-     * @ParamConverter("annotation", class="WallabagAnnotationBundle:Annotation")
-     *
      * @return JsonResponse
      */
-    public function putAnnotationAction(Annotation $annotation, Request $request)
+    public function putAnnotationAction(int $annotation, Request $request)
     {
         $this->validateAuthentication();
 
@@ -86,11 +83,9 @@ class AnnotationRestController extends WallabagRestController
      *      }
      * )
      *
-     * @ParamConverter("annotation", class="WallabagAnnotationBundle:Annotation")
-     *
      * @return JsonResponse
      */
-    public function deleteAnnotationAction(Annotation $annotation)
+    public function deleteAnnotationAction(int $annotation)
     {
         $this->validateAuthentication();
 

src/Wallabag/CoreBundle/Controller/ExportController.php

@@ -6,7 +6,6 @@ use Symfony\Bundle\FrameworkBundle\Controller\Controller;
 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, $id)
     {
         try {
+            $entry = $this->get('wallabag_core.entry_repository')
+                ->find((int) $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')

tests/Wallabag/AnnotationBundle/Controller/AnnotationControllerTest.php

@@ -22,8 +22,6 @@ class AnnotationControllerTest extends WallabagAnnotationTestCase
     }
 
     /**
-     * Test fetching annotations for an entry.
-     *
      * @dataProvider dataForEachAnnotations
      */
     public function testGetAnnotations($prefixUrl)
@@ -35,15 +33,7 @@ class AnnotationControllerTest extends WallabagAnnotationTestCase
             ->findOneByUserName('admin');
         $entry = $em
             ->getRepository('WallabagCoreBundle:Entry')
-            ->findOneByUsernameAndNotArchived('admin');
-
-        $annotation = new Annotation($user);
-        $annotation->setEntry($entry);
-        $annotation->setText('This is my annotation /o/');
-        $annotation->setQuote('content');
-
-        $em->persist($annotation);
-        $em->flush();
+            ->findByUrlAndUserId('http://0.0.0.0/entry1', $user->getId());
 
         if ('annotations' === $prefixUrl) {
             $this->logInAs('admin');
@@ -54,23 +44,44 @@ class AnnotationControllerTest extends WallabagAnnotationTestCase
 
         $content = json_decode($this->client->getResponse()->getContent(), true);
         $this->assertGreaterThanOrEqual(1, $content['total']);
-        $this->assertSame($annotation->getText(), $content['rows'][0]['text']);
-
-        // we need to re-fetch the annotation becase after the flush, it has been "detached" from the entity manager
-        $annotation = $em->getRepository('WallabagAnnotationBundle:Annotation')->findAnnotationById($annotation->getId());
-        $em->remove($annotation);
-        $em->flush();
     }
 
     /**
-     * Test creating an annotation for an entry.
-     *
+     * @dataProvider dataForEachAnnotations
+     */
+    public function testGetAnnotationsFromAnOtherUser($prefixUrl)
+    {
+        $em = $this->client->getContainer()->get('doctrine.orm.entity_manager');
+
+        $otherUser = $em
+            ->getRepository('WallabagUserBundle:User')
+            ->findOneByUserName('bob');
+        $entry = $em
+            ->getRepository('WallabagCoreBundle:Entry')
+            ->findByUrlAndUserId('http://0.0.0.0/entry3', $otherUser->getId());
+
+        if ('annotations' === $prefixUrl) {
+            $this->logInAs('admin');
+        }
+
+        $this->client->request('GET', $prefixUrl . '/' . $entry->getId() . '.json');
+        $this->assertSame(200, $this->client->getResponse()->getStatusCode());
+
+        $content = json_decode($this->client->getResponse()->getContent(), true);
+        $this->assertGreaterThanOrEqual(0, $content['total']);
+    }
+
+    /**
      * @dataProvider dataForEachAnnotations
      */
     public function testSetAnnotation($prefixUrl)
     {
         $em = $this->client->getContainer()->get('doctrine.orm.entity_manager');
 
+        $user = $em
+            ->getRepository('WallabagUserBundle:User')
+            ->findOneByUserName('admin');
+
         if ('annotations' === $prefixUrl) {
             $this->logInAs('admin');
         }
@@ -102,7 +113,7 @@ class AnnotationControllerTest extends WallabagAnnotationTestCase
         /** @var Annotation $annotation */
         $annotation = $em
             ->getRepository('WallabagAnnotationBundle:Annotation')
-            ->findLastAnnotationByPageId($entry->getId(), 1);
+            ->findLastAnnotationByUserId($entry->getId(), $user->getId());
 
         $this->assertSame('my annotation', $annotation->getText());
     }
@@ -195,8 +206,6 @@ class AnnotationControllerTest extends WallabagAnnotationTestCase
     }
 
     /**
-     * Test editing an existing annotation.
-     *
      * @dataProvider dataForEachAnnotations
      */
     public function testEditAnnotation($prefixUrl)
@@ -243,8 +252,31 @@ class AnnotationControllerTest extends WallabagAnnotationTestCase
     }
 
     /**
-     * Test deleting an annotation.
-     *
+     * @dataProvider dataForEachAnnotations
+     */
+    public function testEditAnnotationFromAnOtherUser($prefixUrl)
+    {
+        $em = $this->client->getContainer()->get('doctrine.orm.entity_manager');
+
+        $otherUser = $em
+            ->getRepository('WallabagUserBundle:User')
+            ->findOneByUserName('bob');
+        $entry = $em
+            ->getRepository('WallabagCoreBundle:Entry')
+            ->findByUrlAndUserId('http://0.0.0.0/entry3', $otherUser->getId());
+        $annotation = $em
+            ->getRepository('WallabagAnnotationBundle:Annotation')
+            ->findLastAnnotationByUserId($entry->getId(), $otherUser->getId());
+
+        $headers = ['CONTENT_TYPE' => 'application/json'];
+        $content = json_encode([
+            'text' => 'a modified annotation',
+        ]);
+        $this->client->request('PUT', $prefixUrl . '/' . $annotation->getId() . '.json', [], [], $headers, $content);
+        $this->assertSame(404, $this->client->getResponse()->getStatusCode());
+    }
+
+    /**
      * @dataProvider dataForEachAnnotations
      */
     public function testDeleteAnnotation($prefixUrl)
@@ -287,4 +319,40 @@ class AnnotationControllerTest extends WallabagAnnotationTestCase
 
         $this->assertNull($annotationDeleted);
     }
+
+    /**
+     * @dataProvider dataForEachAnnotations
+     */
+    public function testDeleteAnnotationFromAnOtherUser($prefixUrl)
+    {
+        $em = $this->client->getContainer()->get('doctrine.orm.entity_manager');
+
+        $otherUser = $em
+            ->getRepository('WallabagUserBundle:User')
+            ->findOneByUserName('bob');
+        $entry = $em
+            ->getRepository('WallabagCoreBundle:Entry')
+            ->findByUrlAndUserId('http://0.0.0.0/entry3', $otherUser->getId());
+        $annotation = $em
+            ->getRepository('WallabagAnnotationBundle:Annotation')
+            ->findLastAnnotationByUserId($entry->getId(), $otherUser->getId());
+
+        $user = $em
+            ->getRepository('WallabagUserBundle:User')
+            ->findOneByUserName('admin');
+        $entry = $em
+            ->getRepository('WallabagCoreBundle:Entry')
+            ->findOneByUsernameAndNotArchived('admin');
+
+        if ('annotations' === $prefixUrl) {
+            $this->logInAs('admin');
+        }
+
+        $headers = ['CONTENT_TYPE' => 'application/json'];
+        $content = json_encode([
+            'text' => 'a modified annotation',
+        ]);
+        $this->client->request('DELETE', $prefixUrl . '/' . $annotation->getId() . '.json', [], [], $headers, $content);
+        $this->assertSame(404, $this->client->getResponse()->getStatusCode());
+    }
 }

tests/Wallabag/CoreBundle/Controller/ConfigControllerTest.php

@@ -932,7 +932,7 @@ class ConfigControllerTest extends WallabagCoreTestCase
 
         $annotationsReset = $em
             ->getRepository('WallabagAnnotationBundle:Annotation')
-            ->findAnnotationsByPageId($entry->getId(), $user->getId());
+            ->findByEntryIdAndUserId($entry->getId(), $user->getId());
 
         $this->assertEmpty($annotationsReset, 'Annotations were reset');
 
@@ -1040,7 +1040,7 @@ class ConfigControllerTest extends WallabagCoreTestCase
 
         $annotationsReset = $em
             ->getRepository('WallabagAnnotationBundle:Annotation')
-            ->findAnnotationsByPageId($annotationArchived->getId(), $user->getId());
+            ->findByEntryIdAndUserId($annotationArchived->getId(), $user->getId());
 
         $this->assertEmpty($annotationsReset, 'Annotations were reset');
     }
@@ -1097,7 +1097,7 @@ class ConfigControllerTest extends WallabagCoreTestCase
 
         $annotationsReset = $em
             ->getRepository('WallabagAnnotationBundle:Annotation')
-            ->findAnnotationsByPageId($entry->getId(), $user->getId());
+            ->findByEntryIdAndUserId($entry->getId(), $user->getId());
 
         $this->assertEmpty($annotationsReset, 'Annotations were reset');
     }

tests/Wallabag/CoreBundle/Controller/EntryControllerTest.php

@@ -1493,11 +1493,11 @@ class EntryControllerTest extends WallabagCoreTestCase
                 'zh_CN',
             ],
             'pt_BR' => [
-                'https://politica.estadao.com.br/noticias/eleicoes,campanha-catatonica,70002491983',
+                'https://esportes.r7.com/lance/futebol/victor-hugo-e-matheus-franca-devem-desfalcar-flamengo-no-carioca-22112022',
                 'pt_BR',
             ],
             'es-ES' => [
-                'https://elpais.com/internacional/2022-10-09/ultima-hora-de-la-guerra-en-ucrania-hoy-en-directo.html',
+                'https://elpais.com/internacional/2022-11-03/ultima-hora-de-la-guerra-entre-rusia-y-ucrania-hoy-en-directo.html',
                 'es',
             ],
         ];

tests/Wallabag/CoreBundle/Controller/ExportControllerTest.php

@@ -57,7 +57,7 @@ class ExportControllerTest extends WallabagCoreTestCase
         $this->assertSame(404, $client->getResponse()->getStatusCode());
     }
 
-    public function testBadEntryId()
+    public function testNonExistingEntryId()
     {
         $this->logInAs('admin');
         $client = $this->getClient();
@@ -67,6 +67,21 @@ class ExportControllerTest extends WallabagCoreTestCase
         $this->assertSame(404, $client->getResponse()->getStatusCode());
     }
 
+    public function testForbiddenEntryId()
+    {
+        $this->logInAs('admin');
+        $client = $this->getClient();
+
+        $content = $client->getContainer()
+            ->get('doctrine.orm.entity_manager')
+            ->getRepository('WallabagCoreBundle:Entry')
+            ->findOneByUsernameAndNotArchived('bob');
+
+        $client->request('GET', '/export/' . $content->getId() . '.mobi');
+
+        $this->assertSame(404, $client->getResponse()->getStatusCode());
+    }
+
     public function testEpubExport()
     {
         $this->logInAs('admin');