From 891945034dc08e980d187d705fbaaefedee33724 Mon Sep 17 00:00:00 2001
From: Jeffrey Li <50533338+jelib3an@users.noreply.github.com>
Date: Thu, 20 May 2021 10:04:21 -0400
Subject: [PATCH] Improve signed url signature verification, fixes #37370

---
 src/Illuminate/Routing/UrlGenerator.php      |  6 +-
 tests/Integration/Routing/UrlSigningTest.php | 70 ++++++++++++++++++++
 2 files changed, 73 insertions(+), 3 deletions(-)

diff --git a/src/Illuminate/Routing/UrlGenerator.php b/src/Illuminate/Routing/UrlGenerator.php
index e8e09bd5686c..a8203dd41422 100755
--- a/src/Illuminate/Routing/UrlGenerator.php
+++ b/src/Illuminate/Routing/UrlGenerator.php
@@ -388,9 +388,9 @@ public function hasCorrectSignature(Request $request, $absolute = true)
     {
         $url = $absolute ? $request->url() : '/'.$request->path();
 
-        $original = rtrim($url.'?'.Arr::query(
-            Arr::except($request->query(), 'signature')
-        ), '?');
+        $querystring = ltrim(preg_replace('/(^|&)signature=[^&]+/', '', $request->server->get('QUERY_STRING')), '&');
+
+        $original = rtrim($url.'?'.$querystring, '?');
 
         $signature = hash_hmac('sha256', $original, call_user_func($this->keyResolver));
 
diff --git a/tests/Integration/Routing/UrlSigningTest.php b/tests/Integration/Routing/UrlSigningTest.php
index 8c31100de292..ca8e7b19e9f0 100644
--- a/tests/Integration/Routing/UrlSigningTest.php
+++ b/tests/Integration/Routing/UrlSigningTest.php
@@ -59,6 +59,76 @@ public function testSignedUrlWithUrlWithoutSignatureParameter()
         $this->assertSame('invalid', $this->get('/foo/1')->original);
     }
 
+    public function testSignedUrlWithNullParameter()
+    {
+        Route::get('/foo/{id}', function (Request $request, $id) {
+            return $request->hasValidSignature() ? 'valid' : 'invalid';
+        })->name('foo');
+
+        $this->assertIsString($url = URL::signedRoute('foo', ['id' => 1, 'param']));
+        $this->assertSame('valid', $this->get($url)->original);
+    }
+
+    public function testSignedUrlWithEmptyStringParameter()
+    {
+        Route::get('/foo/{id}', function (Request $request, $id) {
+            return $request->hasValidSignature() ? 'valid' : 'invalid';
+        })->name('foo');
+
+        $this->assertIsString($url = URL::signedRoute('foo', ['id' => 1, 'param' => '']));
+        $this->assertSame('valid', $this->get($url)->original);
+    }
+
+    public function testSignedUrlWithMultipleParameters()
+    {
+        Route::get('/foo/{id}', function (Request $request, $id) {
+            return $request->hasValidSignature() ? 'valid' : 'invalid';
+        })->name('foo');
+
+        $this->assertIsString($url = URL::signedRoute('foo', ['id' => 1, 'param1' => 'value1', 'param2' => 'value2']));
+        $this->assertSame('valid', $this->get($url)->original);
+    }
+
+    public function testSignedUrlWithSignatureTextInKeyOrValue()
+    {
+        Route::get('/foo/{id}', function (Request $request, $id) {
+            return $request->hasValidSignature() ? 'valid' : 'invalid';
+        })->name('foo');
+
+        $this->assertIsString($url = URL::signedRoute('foo', ['id' => 1, 'custom-signature' => 'signature=value']));
+        $this->assertSame('valid', $this->get($url)->original);
+    }
+
+    public function testSignedUrlWithAppendedNullParameterInvalid()
+    {
+        Route::get('/foo/{id}', function (Request $request, $id) {
+            return $request->hasValidSignature() ? 'valid' : 'invalid';
+        })->name('foo');
+
+        $this->assertIsString($url = URL::signedRoute('foo', ['id' => 1]));
+        $this->assertSame('invalid', $this->get($url.'&appended')->original);
+    }
+
+    public function testSignedUrlParametersParsedCorrectly()
+    {
+        Route::get('/foo/{id}', function (Request $request, $id) {
+            return $request->hasValidSignature()
+                && intval($id) === 1
+                && $request->has('paramEmpty')
+                && $request->has('paramEmptyString')
+                && $request->query('paramWithValue') === 'value'
+                ? 'valid'
+                : 'invalid';
+        })->name('foo');
+
+        $this->assertIsString($url = URL::signedRoute('foo', ['id' => 1,
+            'paramEmpty',
+            'paramEmptyString' => '',
+            'paramWithValue' => 'value',
+        ]));
+        $this->assertSame('valid', $this->get($url)->original);
+    }
+
     public function testSignedMiddleware()
     {
         Route::get('/foo/{id}', function (Request $request, $id) {