Patches XSS & CSRF (#1078)

* Patched XSS & CSRF

* Improve markdown rendering

* wip

---------

Co-authored-by: Dries Vints <[email protected]>

Author: OussamaMater

.env.example

@@ -2,7 +2,8 @@ APP_NAME="Laravel.io"
 APP_ENV=local
 APP_KEY=
 APP_DEBUG=true
-APP_URL=http://laravel.io.test
+APP_HOST=laravel.io.test
+APP_URL=http://${APP_HOST}
 
 DB_DATABASE=laravel
 DB_USERNAME=root

app/Http/Middleware/VerifyCsrfToken.php

@@ -3,6 +3,8 @@
 namespace App\Http\Middleware;
 
 use Illuminate\Foundation\Http\Middleware\VerifyCsrfToken as Middleware;
+use Override;
+use Symfony\Component\HttpFoundation\Cookie;
 
 class VerifyCsrfToken extends Middleware
 {
@@ -14,4 +16,21 @@ class VerifyCsrfToken extends Middleware
     protected $except = [
         //
     ];
+
+    #[Override]
+    protected function newCookie($request, $config)
+    {
+        return new Cookie(
+            'XSRF-TOKEN',
+            $request->session()->token(),
+            $this->availableAt(60 * $config['lifetime']),
+            $config['path'],
+            $config['domain'],
+            $config['secure'],
+            true,
+            false,
+            $config['same_site'] ?? null,
+            $config['partitioned'] ?? false
+        );
+    }
 }

app/Livewire/Editor.php

@@ -73,7 +73,7 @@ public function getUsers($query): array
 
     public function getPreviewProperty(): string
     {
-        return replace_links(md_to_html($this->body ?: ''));
+        return md_to_html($this->body ?: '');
     }
 
     public function preview(): void

app/Markdown/LeagueConverter.php

@@ -6,9 +6,8 @@
 
 final class LeagueConverter implements Converter
 {
-    public function __construct(
-        private MarkdownConverter $converter
-    ) {
+    public function __construct(private MarkdownConverter $converter)
+    {
     }
 
     public function toHtml(string $markdown): string

app/Markdown/MarkdownServiceProvider.php

@@ -5,6 +5,8 @@
 use Illuminate\Support\ServiceProvider;
 use League\CommonMark\Environment\Environment;
 use League\CommonMark\Extension\CommonMark\CommonMarkCoreExtension;
+use League\CommonMark\Extension\ExternalLink\ExternalLinkExtension;
+use League\CommonMark\Extension\GithubFlavoredMarkdownExtension;
 use League\CommonMark\Extension\Mention\MentionExtension;
 use League\CommonMark\MarkdownConverter;
 
@@ -15,17 +17,26 @@ public function register(): void
         $this->app->singleton(Converter::class, function () {
             $environment = new Environment([
                 'html_input' => 'escape',
+                'max_nesting_level' => 10,
+                'allow_unsafe_links' => false,
                 'mentions' => [
                     'username' => [
                         'prefix' => '@',
                         'pattern' => '[a-z\d](?:[a-z\d]|-(?=[a-z\d])){0,38}(?!\w)',
                         'generator' => config('app.url').'/user/%s',
                     ],
                 ],
+                'external_link' => [
+                    'internal_hosts' => config('app.host'),
+                    'open_in_new_window' => true,
+                    'nofollow' => 'external',
+                ],
             ]);
 
             $environment->addExtension(new CommonMarkCoreExtension);
+            $environment->addExtension(new GithubFlavoredMarkdownExtension);
             $environment->addExtension(new MentionExtension);
+            $environment->addExtension(new ExternalLinkExtension);
 
             return new LeagueConverter(new MarkdownConverter($environment));
         });

composer.json

@@ -36,8 +36,7 @@
         "spatie/laravel-schedule-monitor": "^3.1",
         "spatie/laravel-sitemap": "^7.1",
         "symfony/http-client": "^7.0",
-        "symfony/mailgun-mailer": "^7.0",
-        "yarri/link-finder": "^2.5"
+        "symfony/mailgun-mailer": "^7.0"
     },
     "require-dev": {
         "fakerphp/faker": "^1.10",

composer.lock

@@ -55,6 +55,8 @@
     |
     */
 
+    'host' => env('APP_HOST', 'localhost'),
+
     'url' => env('APP_URL', 'http://localhost'),
 
     'asset_url' => env('ASSET_URL', null),

config/app.php

@@ -18,6 +18,7 @@
     <php>
         <env name="APP_ENV" value="testing"/>
         <env name="APP_KEY" value="base64:NXoQgjw2ZlOxnGbo5ZRhYgTdM6xLYsgYElNAgcTQJkE="/>
+        <env name="APP_HOST" value="localhost"/>
         <env name="APP_URL" value="http://localhost"/>
         <env name="CACHE_DRIVER" value="array"/>
         <env name="DB_CONNECTION" value="sqlite"/>

phpunit.xml

@@ -22,7 +22,7 @@ function is_active(mixed $routes): bool
 
 if (! function_exists('md_to_html')) {
     /**
-     * Convert Markdown to HTML.
+     * Converts Markdown to a safe HTML string.
      */
     function md_to_html(string $markdown): string
     {
@@ -42,18 +42,6 @@ function route_to_reply_able(mixed $replyAble): string
     }
 }
 
-if (! function_exists('replace_links')) {
-    /**
-     * Convert Standalone Urls to HTML.
-     */
-    function replace_links(string $markdown): string
-    {
-        return (new LinkFinder([
-            'attrs' => ['target' => '_blank', 'rel' => 'nofollow'],
-        ]))->processHtml($markdown);
-    }
-}
-
 if (! function_exists('canonical')) {
     /**
      * Generate a canonical URL to the given route and allowed list of query params.

resources/helpers.php

@@ -2,22 +2,10 @@
 
 use Illuminate\Support\Facades\Blade;
 
-Blade::directive('md', function ($expression) {
-    return "<?php echo md_to_html($expression); ?>";
-});
-
 Blade::directive('error', function ($expression) {
     return "<?php echo \$errors->first($expression, '<span class=\"block text-sm text-red-500 mt-2\">:message</span>'); ?>";
 });
 
-Blade::directive('formGroup', function ($expression) {
-    return "<div class=\"form-group<?php echo \$errors->has($expression) ? ' has-error' : '' ?>\">";
-});
-
-Blade::directive('endFormGroup', function ($expression) {
-    return '</div>';
-});
-
 Blade::directive('title', function ($expression) {
     return "<?php \$title = $expression ?>";
 });

resources/macros/blade.php

@@ -106,7 +106,7 @@ class="w-full bg-center {{ $article->hasHeroImage() ? 'bg-cover' : '' }} bg-gray
                         x-init="$nextTick(function () { highlightCode($el); })"
                         class="prose prose-lg text-gray-800 prose-lio"
                     >
-                        <x-buk-markdown>{!! $article->body() !!}</x-buk-markdown>
+                        <x-buk-markdown>{!! md_to_html($article->body()) !!}</x-buk-markdown>
                     </div>
 
                     @if ($article->isUpdated())

resources/views/articles/show.blade.php

@@ -55,9 +55,8 @@
         class="prose prose-lio max-w-none p-6 break-words"
         x-data="{}"
         x-init="$nextTick(function () { highlightCode($el); })"
-        x-html="{{ json_encode(replace_links(md_to_html($thread->body()))) }}"
-    >
-    </div>
+        x-html="{{ json_encode(md_to_html($thread->body())) }}"
+    ></div>
 
     @if ($thread->isUpdated())
         <div class="text-sm text-gray-900 p-6">

resources/views/components/threads/thread.blade.php

@@ -5,7 +5,7 @@
         x-init="$nextTick(function () { highlightCode($el); }); $watch('edit', function () { highlightCode($el); }); $wire.on('replyEdited', function () { show = true; edit = false; });"
         class="prose prose-lio max-w-none p-6 break-words"
     >
-        {!! replace_links(md_to_html($reply->body())) !!}
+        {!! md_to_html($reply->body()) !!}
     </div>
 
     @can(App\Policies\ReplyPolicy::UPDATE, $reply)

resources/views/livewire/edit-reply.blade.php

@@ -233,7 +233,7 @@
 
     $this->get("/forum/{$thread->slug}")
         ->assertSee(new HtmlString(
-            '<a href="https://github.com/laravelio/laravel.io" rel="nofollow" target="_blank">https://github.com/laravelio/laravel.io</a>'
+            '<a rel="nofollow noopener noreferrer" target="_blank" href="https://github.com/laravelio/laravel.io">https://github.com/laravelio/laravel.io</a>'
         ));
 });
 
@@ -245,7 +245,7 @@
     Reply::factory()->create(['replyable_id' => $thread->id()]);
 
     $this->get("/forum/{$thread->slug()}")
-        ->assertSee(new HtmlString('&quot;&lt;p&gt;&lt;a href=\&quot;https:\/\/github.com\/laravelio\/laravel.io\&quot; rel=\&quot;nofollow\&quot; target=\&quot;_blank\&quot;&gt;https:\/\/github.com\/laravelio\/laravel.io&lt;\/a&gt;'));
+        ->assertSee(new HtmlString('&quot;&lt;p&gt;&lt;a rel=\&quot;nofollow noopener noreferrer\&quot; target=\&quot;_blank\&quot; href=\&quot;https:\/\/github.com\/laravelio\/laravel.io\&quot;&gt;https:\/\/github.com\/laravelio\/laravel.io&lt;\/a&gt;'));
 });
 
 test('an invalid filter defaults to the most recent threads', function () {

tests/Feature/ForumTest.php

@@ -0,0 +1,17 @@
+<?php
+
+use Tests\TestCase;
+
+uses(TestCase::class);
+
+test('converts markdown to safe html', function () {
+    $body = 'Hello, World! ![](image.png).';
+
+    expect(md_to_html($body))->toBe('<p>Hello, World! <img src="image.png" alt="" />.</p>' . "\n");
+});
+
+test('prevents unsafe links', function () {
+    $body = "[Unsafe Link](javascript:alert('Hello'))";
+
+    expect(md_to_html($body))->toBe('<p><a>Unsafe Link</a></p>' . "\n");
+});