[PLUGINS][Reply] Redirect back to previous URL on note reply. Move controller to own class

This should be safe against open redirects, as it doesn't allow redirecting to other domains

plugins/Reply/Controller/Reply.php

@@ -0,0 +1,100 @@
+<?php
+// {{{ License
+// This file is part of GNU social - https://www.gnu.org/software/social
+//
+// GNU social is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// GNU social is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with GNU social.  If not, see <http://www.gnu.org/licenses/>.
+// }}}
+
+/**
+ * @author    Hugo Sales <hugo@hsal.es>
+ * @copyright 2021 Free Software Foundation, Inc http://www.fsf.org
+ * @license   https://www.gnu.org/licenses/agpl.html GNU AGPL v3 or later
+ */
+
+namespace Plugin\Reply\Controller;
+
+use App\Core\Controller;
+use App\Core\DB\DB;
+use App\Core\Form;
+use function App\Core\I18n\_m;
+use App\Core\Log;
+use App\Core\Router\Router;
+use App\Entity\Note;
+use App\Util\Common;
+use App\Util\Exception\ClientException;
+use App\Util\Exception\InvalidFormException;
+use App\Util\Exception\RedirectException;
+use Component\Posting\Posting;
+use Symfony\Component\Form\Extension\Core\Type\FileType;
+use Symfony\Component\Form\Extension\Core\Type\SubmitType;
+use Symfony\Component\Form\Extension\Core\Type\TextareaType;
+use Symfony\Component\HttpFoundation\Request;
+
+class Reply extends Controller
+{
+    /**
+     * Controller for the note reply non-JS page
+     */
+    public function handle(Request $request, string $reply_to)
+    {
+        $user     = Common::ensureLoggedIn();
+        $actor_id = $user->getId();
+        $note     = DB::find('note', ['id' => (int) $reply_to]);
+        if ($note === null || !$note->isVisibleTo($user)) {
+            throw new NoSuchNoteException();
+        }
+
+        $form = Form::create([
+            ['content',     TextareaType::class, ['label' => ' ']],
+            ['attachments', FileType::class,     ['label' => ' ', 'multiple' => true, 'required' => false]],
+            ['replyform',   SubmitType::class,   ['label' => _m('Submit')]],
+        ]);
+
+        $form->handleRequest($request);
+
+        if ($form->isSubmitted()) {
+            $data = $form->getData();
+            if ($form->isValid()) {
+                Posting::storeNote(
+                    $actor_id,
+                    $data['content'],
+                    $data['attachments'],
+                    $is_local = true,
+                    $reply_to,
+                    $repeat_of = null
+                );
+                $return = $this->string('return_to');
+                if (!is_null($return)) {
+                    // Prevent open redirect
+                    if (Router::isAbsolute($return)) {
+                        Log::warning("Actor {$actor_id} attempted to reply to a note and then get redirected to another host, or the URL was invalid ({$return})");
+                        throw new ClientException(_m('Can not redirect to outside the website from here'), 400); // 400 Bad request (deceptive)
+                    } else {
+                        throw new RedirectException(url: $return);
+                    }
+                } else {
+                    throw new RedirectException('root'); // If we don't have a URL to return to, go to the instance root
+                }
+            } else {
+                throw new InvalidFormException();
+            }
+        }
+
+        return [
+            '_template' => 'note/reply.html.twig',
+            'note'      => $note,
+            'reply'     => $form->createView(),
+        ];
+    }
+}

plugins/Reply/Reply.php

@@ -21,27 +21,22 @@
 
 namespace Plugin\Reply;
 
-use App\Core\DB\DB;
 use App\Core\Event;
 use App\Core\Form;
-use function App\Core\I18n\_m;
 use App\Core\Modules\NoteHandlerPlugin;
 use App\Entity\Note;
 use App\Util\Common;
-use App\Util\Exception\InvalidFormException;
 use App\Util\Exception\RedirectException;
-use Component\Posting\Posting;
+use Plugin\Reply\Controller\Reply as ReplyController;
-use Symfony\Component\Form\Extension\Core\Type\FileType;
 use Symfony\Component\Form\Extension\Core\Type\HiddenType;
 use Symfony\Component\Form\Extension\Core\Type\SubmitType;
-use Symfony\Component\Form\Extension\Core\Type\TextareaType;
 use Symfony\Component\HttpFoundation\Request;
 
 class Reply extends NoteHandlerPlugin
 {
     public function onAddRoute($r)
     {
-        $r->connect('note_reply', '/note/reply/{reply_to<\\d*>}', [self::class, 'replyController']);
+        $r->connect('note_reply', '/note/reply/{reply_to<\\d*>}', ReplyController::class);
 
         return Event::next;
     }
@@ -49,6 +44,7 @@ class Reply extends NoteHandlerPlugin
     /**
      * HTML rendering event that adds the reply form as a note action,
      * if a user is logged in
+     *
      * @throws RedirectException
      */
     public function onAddNoteActions(Request $request, Note $note, array &$actions)
@@ -61,7 +57,7 @@ class Reply extends NoteHandlerPlugin
             ['content',     HiddenType::class, ['label' => ' ', 'required' => false]],
             ['attachments', HiddenType::class, ['label' => ' ', 'required' => false]],
             ['note_id',     HiddenType::class, ['data' => $note->getId()]],
-            ['reply', SubmitType::class,
+            ['reply',       SubmitType::class,
                 [
                     'label' => ' ',
                     'attr'  => [
@@ -72,7 +68,7 @@ class Reply extends NoteHandlerPlugin
         ]);
 
         // Handle form
-        $ret = self::noteActionHandle($request, $form, $note, 'reply', function ($note, $data, $user) {
+        $ret = self::noteActionHandle($request, $form, $note, 'reply', function ($note, $data, $user) use ($request) {
             if ($data['content'] !== null) {
                 // JS submitted
                 // TODO Implement in JS
@@ -87,7 +83,7 @@ class Reply extends NoteHandlerPlugin
                 );
             } else {
                 // JS disabled, redirect
-                throw new RedirectException('note_reply', ['reply_to' => $note->getId()]);
+                throw new RedirectException('note_reply', ['reply_to' => $note->getId(), 'return_to' => $request->getRequestUri()]);
 
                 return Event::stop;
             }
@@ -99,47 +95,4 @@ class Reply extends NoteHandlerPlugin
         $actions[] = $form->createView();
         return Event::next;
     }
-
-    /**
-     * Controller for the note reply non-JS page
-     */
-    public function replyController(Request $request, string $reply_to)
-    {
-        $user     = Common::ensureLoggedIn();
-        $actor_id = $user->getId();
-        $note     = DB::find('note', ['id' => (int) $reply_to]);
-        if ($note === null || !$note->isVisibleTo($user)) {
-            throw new NoSuchNoteException();
-        }
-
-        $form = Form::create([
-            ['content',     TextareaType::class, ['label' => ' ']],
-            ['attachments', FileType::class,     ['label' => ' ', 'multiple' => true, 'required' => false]],
-            ['replyform',   SubmitType::class,   ['label' => _m('Submit')]],
-        ]);
-
-        $form->handleRequest($request);
-
-        if ($form->isSubmitted()) {
-            $data = $form->getData();
-            if ($form->isValid()) {
-                Posting::storeNote(
-                    $actor_id,
-                    $data['content'],
-                    $data['attachments'],
-                    $is_local = true,
-                    $reply_to,
-                    $repeat_of = null
-                );
-            } else {
-                throw new InvalidFormException();
-            }
-        }
-
-        return [
-            '_template' => 'note/reply.html.twig',
-            'note'      => $note,
-            'reply'     => $form->createView(),
-        ];
-    }
 }

src/Util/Exception/RedirectException.php

@@ -27,11 +27,9 @@ class RedirectException extends Exception
 {
     public ?RedirectResponse $redirect_response = null;
 
-    public function __construct(string $url_id = '', array $args = [], $message = '', $code = 302, Exception $previous_exception = null)
+    public function __construct(string $url_id = '', array $args = [], $message = '', $code = 302, ?string $url = null, ?Exception $previous_exception = null)
     {
-        if (!empty($url_id)) {
+        $this->redirect_response = new RedirectResponse($url ?? Router::url($url_id, $args));
-            $this->redirect_response = new RedirectResponse(Router::url($url_id, $args));
-        }
         parent::__construct($message, $code, $previous_exception);
     }
 }