SECURITY-5146: encode plus sign only when content-type set to applica…

…tion/x-www-form-urlencoded Co-authored-by: David Szigecsan <[email protected]>
emartech · Nov 18, 2021 · c3c1dc6 · c3c1dc6
1 parent 7c63574
commit c3c1dc6
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 6 deletions.
diff --git a/src/Escher/RequestCanonicalizer.php b/src/Escher/RequestCanonicalizer.php
@@ -11,7 +11,7 @@ public static function canonicalize($method, $requestUri, $payload, $rawHeaders,
         $lines = array();
         $lines[] = strtoupper($method);
         $lines[] = self::normalizePath($path);
-        $lines[] = self::urlEncodeQueryString($query);
+        $lines[] = self::urlEncodeQueryString($query, $rawHeaders);
 
         sort($headersToSign);
         $lines = array_merge($lines, self::canonicalizeHeaders($rawHeaders, $headersToSign));
@@ -24,7 +24,7 @@ public static function canonicalize($method, $requestUri, $payload, $rawHeaders,
         return implode("\n", $lines);
     }
 
-    public static function urlEncodeQueryString($query)
+    public static function urlEncodeQueryString($query, $headers)
     {
         if (empty($query)) {
             return '';
@@ -40,8 +40,8 @@ public static function urlEncodeQueryString($query)
             $keyValues[0] = urldecode($keyValues[0]);
             $keyValues[1] = urldecode($keyValues[1]);
             $encodedParts[] = implode('=', array(
-                self::rawUrlEncode(str_replace('+', ' ', $keyValues[0])),
-                self::rawUrlEncode(str_replace('+', ' ', $keyValues[1])),
+                self::rawUrlEncode($keyValues[0], $headers),
+                self::rawUrlEncode($keyValues[1], $headers),
             ));
         }
         sort($encodedParts);
@@ -93,9 +93,13 @@ private static function canonicalizeHeaders($rawHeaders, array $headersToSign)
         return $result;
     }
 
-    private static function rawUrlEncode($urlComponent)
+    private static function rawUrlEncode($urlComponent, $headers)
     {
-        $result = rawurlencode($urlComponent);
+        if(strpos($headers, "application/x-www-form-urlencoded")) {
+            $result = rawurlencode(str_replace('+', ' ', $urlComponent));
+        } else {
+            $result = rawurlencode($urlComponent);
+        }
         if (version_compare(PHP_VERSION, '5.3.4') === -1) {
             $result = str_replace('%7E', '~', $result);
         }

diff --git a/test/unit/RequestCanonicalizerTest.php b/test/unit/RequestCanonicalizerTest.php
@@ -0,0 +1,28 @@
+<?php
+
+use Escher\RequestCanonicalizer;
+
+
+class RequestCanonicalizerTest extends TestBase
+{
+    /**
+     * @test
+     */
+    public function urlEncodeQueryStringShouldNotReplacePlusSign()
+    {
+        $query = "email=test%2Bbayxd%40gmail.com";
+        $result = RequestCanonicalizer::urlEncodeQueryString($query, "application/json");
+        $this->assertEquals($query, $result);
+    }
+
+    /**
+     * @test
+     */
+    public function  urlEncodeQueryStringShouldReplacePlusSignWithSplace()
+    {
+        $query = "email=test%2Bbayxd%40gmail.com";
+        $expected = "email=test%20bayxd%40gmail.com";
+        $result = RequestCanonicalizer::urlEncodeQueryString($query, "application/x-www-form-urlencoded");
+        $this->assertEquals($expected, $result);
+    }
+}