update: remove XSS cleaner and remove XSS vulnerabilities

We've been mostly relying on the 3rd party xss cleaner to make sure user submitted content is clean. This PR fixes up any leftover holes in the bbcode parser that allow xss vulnerabilities, and as a result, the 3rd party library isn't needed anymore. It cleans responsibly by first, running `htmlspecialchars()` over the content, followed by validating any untrusted input used inside html attributes. It validates urls by returning them as redirects relying on the browser not supporting executable protocols (like `javascript:`).

app/Helpers/Bbcode.php

@@ -15,6 +15,18 @@
 
 class Bbcode
 {
+    /**
+     * @var array<
+     *     string,
+     *     array{
+     *         openBbcode: string,
+     *         closeBbcode: string,
+     *         openHtml: string,
+     *         closeHtml: string,
+     *         block: boolean
+     *     }
+     * > $parsers.
+     */
     private array $parsers = [
         'h1' => [
             'openBbcode'  => '/^\[h1\]/i',
@@ -136,19 +148,12 @@ class Bbcode
             'block'       => true,
         ],
         'namedquote' => [
-            'openBbcode'  => '/^\[quote=([^<>"]*?)\]/i',
+            'openBbcode'  => '/^\[quote=(.*?)\]/i',
             'closeBbcode' => '[/quote]',
             'openHtml'    => '<blockquote><i class="fas fa-quote-left"></i> <cite>Quoting $1:</cite><p>',
             'closeHtml'   => '</p></blockquote>',
             'block'       => true,
         ],
-        'namedlink' => [
-            'openBbcode'  => '/^\[url=(.*?)\]/i',
-            'closeBbcode' => '[/url]',
-            'openHtml'    => '<a href="$1">',
-            'closeHtml'   => '</a>',
-            'block'       => false,
-        ],
         'orderedlistnumerical' => [
             'openBbcode'  => '/^\[list=1\]/i',
             'closeBbcode' => '[/list]',
@@ -259,46 +264,54 @@ class Bbcode
     /**
      * Parses the BBCode string.
      */
-    public function parse($source, $replaceLineBreaks = true): string
+    public function parse(?string $source, bool $replaceLineBreaks = true): string
     {
+        $source ??= '';
+        $source = htmlspecialchars($source, ENT_QUOTES, 'UTF-8');
+
         // Replace all void elements since they don't have closing tags
-        $source = str_replace('[*]', '<li>', (string) $source);
+        $source = str_replace('[*]', '<li>', $source);
         $source = preg_replace_callback(
             '/\[url\](.*?)\[\/url\]/i',
-            fn ($matches) => '<a href="'.htmlspecialchars($matches[1]).'">'.htmlspecialchars($matches[1]).'</a>',
+            fn ($matches) => '<a href="'.route('exit', ['url' => $matches[1]]).'">'.$matches[1].'</a>',
+            $source
+        );
+        $source = preg_replace_callback(
+            '/\[url=(.*?)\](.*?)\[\/url\]/i',
+            fn ($matches) => '<a href="'.route('exit', ['url' => $matches[1]]).'">'.$matches[2].'</a>',
             $source
         );
         $source = preg_replace_callback(
             '/\[img\](.*?)\[\/img\]/i',
-            fn ($matches) => '<img src="'.htmlspecialchars($matches[1]).'" loading="lazy" class="img-responsive" style="display: inline !important;">',
+            fn ($matches) => '<img src="'.route('exit', ['url' => $matches[1]]).'" loading="lazy" class="img-responsive" style="display: inline !important;">',
             $source
         );
         $source = preg_replace_callback(
             '/\[img width=(\d+)\](.*?)\[\/img\]/i',
-            fn ($matches) => '<img src="'.htmlspecialchars($matches[2]).'" loading="lazy" width="'.$matches[1].'px">',
+            fn ($matches) => '<img src="'.route('exit', ['url' => $matches[2]]).'" loading="lazy" width="'.$matches[1].'px">',
             $source
         );
         $source = preg_replace_callback(
             '/\[img=(\d+)(?:x\d+)?\](.*?)\[\/img\]/i',
-            fn ($matches) => '<img src="'.htmlspecialchars($matches[2]).'" loading="lazy" width="'.$matches[1].'px">',
+            fn ($matches) => '<img src="'.route('exit', ['url' => $matches[2]]).'" loading="lazy" width="'.$matches[1].'px">',
             $source
         );
 
         // Youtube elements need to be replaced like this because the content inside the two tags
         // has to be moved into an html attribute
         $source = preg_replace_callback(
-            '/\[youtube\](.*?)\[\/youtube\]/i',
-            fn ($matches) => '<iframe width="560" height="315" src="https://www.youtube-nocookie.com/embed/'.htmlspecialchars($matches[1]).'?rel=0" allow="autoplay; encrypted-media" allowfullscreen></iframe>',
+            '/\[youtube\]([a-z0-9_-]{11})\[\/youtube\]/i',
+            fn ($matches) => '<iframe width="560" height="315" src="https://www.youtube-nocookie.com/embed/'.$matches[1].'?rel=0" allow="autoplay; encrypted-media" allowfullscreen></iframe>',
             $source
         );
         $source = preg_replace_callback(
-            '/\[video\](.*?)\[\/video\]/i',
-            fn ($matches) => '<iframe width="560" height="315" src="https://www.youtube-nocookie.com/embed/'.htmlspecialchars($matches[1]).'?rel=0" allow="autoplay; encrypted-media" allowfullscreen></iframe>',
+            '/\[video\]([a-z0-9_-]{11})\[\/video\]/i',
+            fn ($matches) => '<iframe width="560" height="315" src="https://www.youtube-nocookie.com/embed/'.$matches[1].'?rel=0" allow="autoplay; encrypted-media" allowfullscreen></iframe>',
             $source
         );
         $source = preg_replace_callback(
-            '/\[video="youtube"\](.*?)\[\/video\]/i',
-            fn ($matches) => '<iframe width="560" height="315" src="https://www.youtube-nocookie.com/embed/'.htmlspecialchars($matches[1]).'?rel=0" allow="autoplay; encrypted-media" allowfullscreen></iframe>',
+            '/\[video=&quot;youtube&quot;\]([a-z0-9_-]{11})\[\/video\]/i',
+            fn ($matches) => '<iframe width="560" height="315" src="https://www.youtube-nocookie.com/embed/'.$matches[1].'?rel=0" allow="autoplay; encrypted-media" allowfullscreen></iframe>',
             $source
         );
 
@@ -309,7 +322,7 @@ public function parse($source, $replaceLineBreaks = true): string
             function ($matches) {
                 $comparates = preg_split('/\s*,\s*/', $matches[1]);
                 $urls = preg_split('/\s*(?:,|\s)\s*/', $matches[2]);
-                $validatedUrls = collect($urls)->filter(fn ($url) => filter_var($url, FILTER_VALIDATE_URL));
+                $validatedUrls = collect($urls)->map(fn ($url) => route('exit', ['url' => $url]));
                 $chunkedUrls = $validatedUrls->chunk(\count($comparates));
                 $html = view('partials.comparison', ['comparates' => $comparates, 'urls' => $chunkedUrls])->render();
                 $html = preg_replace('/\s+/', ' ', $html);
@@ -344,15 +357,15 @@ function ($matches) {
             if ($source[$index + 1] === '/' && ! empty($openedElements)) {
                 $name = array_pop($openedElements);
                 $el = $this->parsers[$name];
-                $tag = substr((string) $source, $index, \strlen((string) $el['closeBbcode']));
+                $tag = substr((string) $source, $index, \strlen($el['closeBbcode']));
 
                 // Replace bbcode tag with html tag if found tag matches expected tag,
                 // otherwise return the expected element's to the stack
-                if (strcasecmp($tag, (string) $el['closeBbcode']) === 0) {
-                    $source = substr_replace((string) $source, (string) $el['closeHtml'], $index, \strlen((string) $el['closeBbcode']));
+                if (strcasecmp($tag, $el['closeBbcode']) === 0) {
+                    $source = substr_replace((string) $source, $el['closeHtml'], $index, \strlen($el['closeBbcode']));
 
                     if ($replaceLineBreaks === true && $el['block'] === true) {
-                        $this->handleBlockElementSpacing($source, $index, $index, $index + \strlen((string) $el['closeHtml']) - 1);
+                        $this->handleBlockElementSpacing($source, $index, $index, $index + \strlen($el['closeHtml']) - 1);
                     }
                 } else {
                     $openedElements[] = $name;
@@ -365,7 +378,7 @@ function ($matches) {
                     // The opening bbcode tag uses the regex `^` character to make
                     // sure only the beginning of $remainingText is matched
                     if (preg_match($el['openBbcode'], $remainingText, $matches) === 1) {
-                        $replacement = preg_replace($el['openBbcode'], (string) $el['openHtml'], $matches[0]);
+                        $replacement = preg_replace($el['openBbcode'], $el['openHtml'], $matches[0]);
                         $source = substr_replace((string) $source, $replacement, $index, \strlen($matches[0]));
 
                         if ($replaceLineBreaks === true && $el['block'] === true) {

app/Http/Controllers/ExitController.php

@@ -0,0 +1,27 @@
+<?php
+/**
+ * NOTICE OF LICENSE.
+ *
+ * UNIT3D Community Edition is open-sourced software licensed under the GNU Affero General Public License v3.0
+ * The details is bundled with this project in the file LICENSE.txt.
+ *
+ * @project    UNIT3D Community Edition
+ *
+ * @author     Roardom <roardom@protonmail.com>
+ * @license    https://www.gnu.org/licenses/agpl-3.0.en.html/ GNU Affero General Public License v3.0
+ */
+
+namespace App\Http\Controllers;
+
+use Illuminate\Http\Request;
+
+class ExitController extends Controller
+{
+    /**
+     * Redirect to an external URL.
+     */
+    public function __invoke(Request $request): \Illuminate\Routing\Redirector|\Illuminate\Http\RedirectResponse
+    {
+        return redirect()->away($request->string('url'));
+    }
+}

app/Http/Livewire/BbcodeInput.php

@@ -35,7 +35,7 @@ final public function mount($name, $label, $required = false, $content = null):
         $this->name = $name;
         $this->label = $label;
         $this->isRequired = $required;
-        $this->contentBbcode = $content === null ? (old($name) ?? '') : htmlspecialchars_decode((string) $content);
+        $this->contentBbcode = $content ?? old($name) ?? '';
     }
 
     final public function updatedIsPreviewEnabled(): void

app/Http/Livewire/Comment.php

@@ -31,7 +31,6 @@
 use App\Repositories\ChatRepository;
 use Illuminate\Support\Facades\Notification;
 use Livewire\Component;
-use voku\helper\AntiXSS;
 
 class Comment extends Component
 {
@@ -94,7 +93,7 @@ final public function updatedIsEditing($isEditing): void
     final public function editComment(): void
     {
         if (auth()->id() == $this->comment->user_id || auth()->user()->group->is_modo) {
-            $this->comment->update((new AntiXSS())->xss_clean($this->editState));
+            $this->comment->update($this->editState);
             $this->isEditing = false;
         } else {
             $this->dispatchBrowserEvent('error', ['type' => 'error',  'message' => 'Permission Denied!']);
@@ -127,7 +126,7 @@ final public function postReply(): void
             'replyState.content' => 'required',
         ]);
 
-        $reply = $this->comment->children()->make((new AntiXSS())->xss_clean($this->replyState));
+        $reply = $this->comment->children()->make($this->replyState);
         $reply->user()->associate(auth()->user());
         $reply->commentable()->associate($this->comment->commentable);
         $reply->anon = $this->anon;

app/Http/Livewire/Comments.php

@@ -33,7 +33,6 @@
 use Illuminate\Support\Facades\Notification;
 use Livewire\Component;
 use Livewire\WithPagination;
-use voku\helper\AntiXSS;
 
 class Comments extends Component
 {
@@ -101,7 +100,7 @@ final public function postComment(): void
             'newCommentState.content' => 'required',
         ]);
 
-        $comment = $this->model->comments()->make((new AntiXSS())->xss_clean($this->newCommentState));
+        $comment = $this->model->comments()->make($this->newCommentState);
         $comment->user()->associate($this->user);
         $comment->anon = $this->anon;
         $comment->save();

app/Models/Article.php

@@ -18,7 +18,6 @@
 use App\Traits\Auditable;
 use Illuminate\Database\Eloquent\Factories\HasFactory;
 use Illuminate\Database\Eloquent\Model;
-use voku\helper\AntiXSS;
 
 class Article extends Model
 {
@@ -82,14 +81,6 @@ public function getBrief(int $length = 20, bool $ellipses = true, bool $stripHtm
         return $trimmedText;
     }
 
-    /**
-     * Set The Articles Content After Its Been Purified.
-     */
-    public function setContentAttribute(?string $value): void
-    {
-        $this->attributes['content'] = htmlspecialchars((new AntiXSS())->xss_clean($value), ENT_NOQUOTES);
-    }
-
     /**
      * Parse Content And Return Valid HTML.
      */

app/Models/Message.php

@@ -16,7 +16,6 @@
 use App\Helpers\Bbcode;
 use Illuminate\Database\Eloquent\Factories\HasFactory;
 use Illuminate\Database\Eloquent\Model;
-use voku\helper\AntiXSS;
 
 class Message extends Model
 {
@@ -75,14 +74,6 @@ public function chatroom(): \Illuminate\Database\Eloquent\Relations\BelongsTo
         return $this->belongsTo(Chatroom::class);
     }
 
-    /**
-     * Set The Chat Message After Its Been Purified.
-     */
-    public function setMessageAttribute(string $value): void
-    {
-        $this->attributes['message'] = htmlspecialchars((new AntiXSS())->xss_clean($value), ENT_NOQUOTES);
-    }
-
     /**
      * Parse Content And Return Valid HTML.
      */

app/Models/Playlist.php

@@ -18,7 +18,6 @@
 use App\Traits\Auditable;
 use Illuminate\Database\Eloquent\Factories\HasFactory;
 use Illuminate\Database\Eloquent\Model;
-use voku\helper\AntiXSS;
 
 class Playlist extends Model
 {
@@ -58,14 +57,6 @@ public function comments(): \Illuminate\Database\Eloquent\Relations\MorphMany
         return $this->morphMany(Comment::class, 'commentable');
     }
 
-    /**
-     * Set The Playlists Description After It's Been Purified.
-     */
-    public function setDescriptionAttribute(?string $value): void
-    {
-        $this->attributes['description'] = htmlspecialchars((new AntiXSS())->xss_clean($value), ENT_NOQUOTES);
-    }
-
     /**
      * Parse Description And Return Valid HTML.
      */

app/Models/Post.php

@@ -18,7 +18,6 @@
 use App\Traits\Auditable;
 use Illuminate\Database\Eloquent\Factories\HasFactory;
 use Illuminate\Database\Eloquent\Model;
-use voku\helper\AntiXSS;
 
 class Post extends Model
 {
@@ -104,14 +103,6 @@ public function authorTopics(): \Illuminate\Database\Eloquent\Relations\HasMany
         return $this->hasMany(Topic::class, 'first_post_user_id', 'user_id');
     }
 
-    /**
-     * Set The Posts Content After Its Been Purified.
-     */
-    public function setContentAttribute(?string $value): void
-    {
-        $this->attributes['content'] = htmlspecialchars((new AntiXSS())->xss_clean($value), ENT_NOQUOTES);
-    }
-
     /**
      * Parse Content And Return Valid HTML.
      */

app/Models/PrivateMessage.php

@@ -17,7 +17,6 @@
 use App\Helpers\Linkify;
 use Illuminate\Database\Eloquent\Factories\HasFactory;
 use Illuminate\Database\Eloquent\Model;
-use voku\helper\AntiXSS;
 
 class PrivateMessage extends Model
 {
@@ -76,14 +75,6 @@ public function replyRecursive(): \Illuminate\Database\Eloquent\Relations\HasOne
         return $this->reply()->with('replyRecursive');
     }
 
-    /**
-     * Set The PM Message After Its Been Purified.
-     */
-    public function setMessageAttribute(string $value): void
-    {
-        $this->attributes['message'] = htmlspecialchars((new AntiXSS())->xss_clean($value), ENT_NOQUOTES);
-    }
-
     /**
      * Parse Content And Return Valid HTML.
      */

app/Models/Torrent.php

@@ -25,7 +25,6 @@
 use App\Traits\TorrentFilter;
 use Illuminate\Database\Eloquent\Factories\HasFactory;
 use Illuminate\Database\Eloquent\Model;
-use voku\helper\AntiXSS;
 
 /**
  * @property string $info_hash
@@ -318,14 +317,6 @@ public function resurrections(): \Illuminate\Database\Eloquent\Relations\HasMany
         return $this->hasMany(Resurrection::class);
     }
 
-    /**
-     * Set The Torrents Description After Its Been Purified.
-     */
-    public function setDescriptionAttribute(?string $value): void
-    {
-        $this->attributes['description'] = htmlspecialchars((new AntiXSS())->xss_clean($value), ENT_NOQUOTES);
-    }
-
     /**
      * Parse Description And Return Valid HTML.
      */

app/Models/TorrentRequest.php

@@ -19,7 +19,6 @@
 use App\Traits\TorrentFilter;
 use Illuminate\Database\Eloquent\Factories\HasFactory;
 use Illuminate\Database\Eloquent\Model;
-use voku\helper\AntiXSS;
 
 class TorrentRequest extends Model
 {
@@ -159,14 +158,6 @@ public function claim(): \Illuminate\Database\Eloquent\Relations\HasOne
         return $this->hasOne(TorrentRequestClaim::class, 'request_id');
     }
 
-    /**
-     * Set The Requests Description After Its Been Purified.
-     */
-    public function setDescriptionAttribute(?string $value): void
-    {
-        $this->attributes['description'] = htmlspecialchars((new AntiXSS())->xss_clean($value), ENT_NOQUOTES);
-    }
-
     /**
      * Parse Description And Return Valid HTML.
      */