Handle edge-case reported in #4

paragonie · Jun 13, 2016 · 802a310 · 802a310
1 parent 83f4c00
commit 802a310
Show file tree

Hide file tree

Showing 3 changed files with 74 additions and 24 deletions.
diff --git a/src/Base32.php b/src/Base32.php
@@ -176,24 +176,24 @@ protected static function doDecode(string $src, bool $upper = false, bool $stric
         if ($srcLen === 0) {
             return '';
         }
-        if (($srcLen & 7) === 0) {
-            for ($j = 0; $j < 7; ++$j) {
-                if ($src[$srcLen - 1] === '=') {
-                    $srcLen--;
-                } else {
-                    break;
+        if ($strictPadding) {
+            if (($srcLen & 7) === 0) {
+                for ($j = 0; $j < 7; ++$j) {
+                    if ($src[$srcLen - 1] === '=') {
+                        $srcLen--;
+                    } else {
+                        break;
+                    }
                 }
             }
-        }
-        if (($srcLen & 7) === 1) {
-            if ($strictPadding) {
+            if (($srcLen & 7) === 1) {
                 throw new \RangeException(
                     'Incorrect padding'
                 );
-            } else {
-                $src = \rtrim($src, '=');
-                $srcLen = Binary::safeStrlen($src);
             }
+        } else {
+            $src = \rtrim($src, '=');
+            $srcLen = Binary::safeStrlen($src);
         }
 
         $err = 0;

diff --git a/src/Base64.php b/src/Base64.php
@@ -94,23 +94,29 @@ public static function decode(string $src, bool $strictPadding = false): string
         if ($srcLen === 0) {
             return '';
         }
-        if (($srcLen & 3) === 0) {
-            if ($src[$srcLen - 1] === '=') {
-                $srcLen--;
+
+        if ($strictPadding) {
+            if (($srcLen & 3) === 0) {
                 if ($src[$srcLen - 1] === '=') {
                     $srcLen--;
+                    if ($src[$srcLen - 1] === '=') {
+                        $srcLen--;
+                    }
                 }
             }
-        }
-        if (($srcLen & 3) === 1) {
-            if ($strictPadding) {
+            if (($srcLen & 3) === 1) {
+                throw new \RangeException(
+                    'Incorrect padding'
+                );
+            }
+            if ($src[$srcLen - 1] === '=') {
                 throw new \RangeException(
                     'Incorrect padding'
                 );
-            } else {
-                $src = \rtrim($src, '=');
-                $srcLen = Binary::safeStrlen($src);
             }
+        } else {
+            $src = \rtrim($src, '=');
+            $srcLen = Binary::safeStrlen($src);
         }
 
         $err = 0;
@@ -135,22 +141,25 @@ public static function decode(string $src, bool $strictPadding = false): string
         if ($i < $srcLen) {
             $chunk = \unpack('C*', Binary::safeSubstr($src, $i, $srcLen - $i));
             $c0 = static::decode6Bits($chunk[1]);
-            $c1 = static::decode6Bits($chunk[2]);
 
             if ($i + 2 < $srcLen) {
+                $c1 = static::decode6Bits($chunk[2]);
                 $c2 = static::decode6Bits($chunk[3]);
                 $dest .= \pack(
                     'CC',
                     ((($c0 << 2) | ($c1 >> 4)) & 0xff),
                     ((($c1 << 4) | ($c2 >> 2)) & 0xff)
                 );
                 $err |= ($c0 | $c1 | $c2) >> 8;
-            } elseif($i + 1 < $srcLen) {
+            } elseif ($i + 1 < $srcLen) {
+                $c1 = static::decode6Bits($chunk[2]);
                 $dest .= \pack(
                     'C',
                     ((($c0 << 2) | ($c1 >> 4)) & 0xff)
                 );
                 $err |= ($c0 | $c1) >> 8;
+            } elseif ($i < $srcLen && $strictPadding) {
+                $err |= 1;
             }
         }
         if ($err !== 0) {

diff --git a/tests/Base64Test.php b/tests/Base64Test.php
@@ -26,9 +26,50 @@ public function testRandom()
                 $unpadded = \rtrim($enc, '=');
                 $this->assertSame(
                     $random,
-                    Base64::decode($unpadded, true)
+                    Base64::decode($unpadded)
                 );
             }
         }
+        $str = 'MIIFzzCCBLegAwIBAgIDAfdlMA0GCSqGSIb3DQEBBQUAMHMxCzAJBgNVBAYTAlBM' .
+            'MSgwJgYDVQQKDB9LcmFqb3dhIEl6YmEgUm96bGljemVuaW93YSBTLkEuMSQwIgYDVQQ' .
+            'DDBtDT1BFIFNaQUZJUiAtIEt3YWxpZmlrb3dhbnkxFDASBgNVBAUTC05yIHdwaXN1Oi' .
+            'A2MB4XDTExMTEwOTA2MDAwMFoXDTEzMTEwOTA2MDAwMFowgdkxCzAJBgNVBAYTAlBMM' .
+            'RwwGgYDVQQKDBNVcnrEhWQgTWlhc3RhIEdkeW5pMRswGQYDVQQFExJQRVNFTDogNjEw' .
+            'NjA2MDMxMTgxGTAXBgNVBAMMEEplcnp5IFByemV3b3Jza2kxTzBNBgNVBBAwRgwiQWw' .
+            'uIE1hcnN6YcWCa2EgUGnFgnN1ZHNraWVnbyA1Mi81NAwNODEtMzgyIEdkeW5pYQwGUG' .
+            '9sc2thDAlwb21vcnNraWUxDjAMBgNVBCoMBUplcnp5MRMwEQYDVQQEDApQcnpld29yc' .
+            '2tpMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQCMm5vjGqHPthJCMqKpqssSISRo' .
+            's0PYDTcEQzyyurfX67EJWKtZj6HNwuDMEGJ02iBNZfjUl7r8dIi28bSKhNlsfycXZKY' .
+            'RcIjp0+r5RqtR2auo9GQ6veKb61DEAGIqaR+uLLcJVTHCu0w9oXLGbRlGth5eNoj03C' .
+            'xXVAH2IfhbNwIDAQABo4IChzCCAoMwDAYDVR0TAQH/BAIwADCCAUgGA1UdIAEB/wSCA' .
+            'TwwggE4MIIBNAYJKoRoAYb3IwEBMIIBJTCB3QYIKwYBBQUHAgIwgdAMgc1EZWtsYXJh' .
+            'Y2phIHRhIGplc3Qgb8Wbd2lhZGN6ZW5pZW0gd3lkYXdjeSwgxbxlIHRlbiBjZXJ0eWZ' .
+            'pa2F0IHpvc3RhxYIgd3lkYW55IGpha28gY2VydHlmaWthdCBrd2FsaWZpa293YW55IH' .
+            'pnb2RuaWUgeiB3eW1hZ2FuaWFtaSB1c3Rhd3kgbyBwb2RwaXNpZSBlbGVrdHJvbmlje' .
+            'm55bSBvcmF6IHRvd2FyenlzesSFY3ltaSBqZWogcm96cG9yesSFZHplbmlhbWkuMEMG' .
+            'CCsGAQUFBwIBFjdodHRwOi8vd3d3Lmtpci5jb20ucGwvY2VydHlmaWthY2phX2tsdWN' .
+            '6eS9wb2xpdHlrYS5odG1sMAkGA1UdCQQCMAAwIQYDVR0RBBowGIEWai5wcnpld29yc2' .
+            'tpQGdkeW5pYS5wbDAOBgNVHQ8BAf8EBAMCBkAwgZ4GA1UdIwSBljCBk4AU3TGldJXip' .
+            'N4oGS3ZYmnBDMFs8gKhd6R1MHMxCzAJBgNVBAYTAlBMMSgwJgYDVQQKDB9LcmFqb3dh' .
+            'IEl6YmEgUm96bGljemVuaW93YSBTLkEuMSQwIgYDVQQDDBtDT1BFIFNaQUZJUiAtIEt' .
+            '3YWxpZmlrb3dhbnkxFDASBgNVBAUTC05yIHdwaXN1OiA2ggJb9jBIBgNVHR8EQTA/MD' .
+            '2gO6A5hjdodHRwOi8vd3d3Lmtpci5jb20ucGwvY2VydHlmaWthY2phX2tsdWN6eS9DU' .
+            'kxfT1pLMzIuY3JsMA0GCSqGSIb3DQEBBQUAA4IBAQBYPIqnAreyeql7/opJjcar/qWZ' .
+            'y9ruhB2q0lZFsJOhwgMnbQXzp/4vv93YJqcHGAXdHP6EO8FQX47mjo2ZKQmi+cIHJHL' .
+            'ONdX/3Im+M17V0iNAh7Z1lOSfTRT+iiwe/F8phcEaD5q2RmvYusR7zXZq/cLL0If0hX' .
+            'oPZ/EHQxjN8pxzxiUx6bJAgturnIMEfRNesxwghdr1dkUjOhGLf3kHVzgM6j3VAM7oF' .
+            'mMUb5y5s96Bzl10DodWitjOEH0vvnIcsppSxH1C1dCAi0o9f/1y2XuLNhBNHMAyTqpY' .
+            'PX8Yvav1c+Z50OMaSXHAnTa20zv8UtiHbaAhwlifCelUMj93S';
+
+        try {
+            Base64::decode($str, true);
+            $this->fail('Strict padding not enforced');
+        } catch (\Exception $ex) {
+
+            $this->assertSame(
+                Base64::decode($str),
+                \base64_decode($str)
+            );
+        }
     }
 }