buffer: fix DoS vector in atob

nodejs · Feb 5, 2024 · 527e435 · 527e435
1 parent 9448c61
commit 527e435
Show file tree

Hide file tree

Showing 2 changed files with 97 additions and 31 deletions.
diff --git a/lib/buffer.js b/lib/buffer.js
@@ -23,10 +23,8 @@
 
 const {
   Array,
-  ArrayFrom,
   ArrayIsArray,
   ArrayPrototypeForEach,
-  ArrayPrototypeIndexOf,
   MathFloor,
   MathMin,
   MathTrunc,
@@ -1255,35 +1253,41 @@ function btoa(input) {
     throw new ERR_MISSING_ARGS('input');
   }
   input = `${input}`;
+  let acc = 0;
   for (let n = 0; n < input.length; n++) {
-    if (input[n].charCodeAt(0) > 0xff)
-      throw lazyDOMException('Invalid character', 'InvalidCharacterError');
+    acc |= StringPrototypeCharCodeAt(input, n);
+  }
+  if (acc & ~0xff) {
+    throw lazyDOMException('Invalid character', 'InvalidCharacterError');
   }
   const buf = Buffer.from(input, 'latin1');
   return buf.toString('base64');
 }
 
 // Refs: https://infra.spec.whatwg.org/#forgiving-base64-decode
+//       https://infra.spec.whatwg.org/#ascii-whitespace
+// Valid Characters: [\t\n\f\r +/0-9=A-Za-z]
+// Lookup table (-1 = invalid, 0 = valid)
+/* eslint-disable no-multi-spaces, indent */
 const kForgivingBase64AllowedChars = [
-  // ASCII whitespace
-  // Refs: https://infra.spec.whatwg.org/#ascii-whitespace
-  0x09, 0x0A, 0x0C, 0x0D, 0x20,
-
-  // Uppercase letters
-  ...ArrayFrom({ length: 26 }, (_, i) => StringPrototypeCharCodeAt('A') + i),
-
-  // Lowercase letters
-  ...ArrayFrom({ length: 26 }, (_, i) => StringPrototypeCharCodeAt('a') + i),
-
-  // Decimal digits
-  ...ArrayFrom({ length: 10 }, (_, i) => StringPrototypeCharCodeAt('0') + i),
-
-  0x2B, // +
-  0x2F, // /
-  0x3D, // =
+  -1, -1, -1, -1, -1, -1, -1, -1,
+  -1,  0,  0, -1,  0,  0, -1, -1,
+  -1, -1, -1, -1, -1, -1, -1, -1,
+  -1, -1, -1, -1, -1, -1, -1, -1,
+   0, -1, -1, -1, -1, -1, -1, -1,
+  -1, -1, -1,  0, -1, -1, -1,  0,
+   0,  0,  0,  0,  0,  0,  0,  0,
+   0,  0, -1, -1, -1,  0, -1, -1,
+  -1,  0,  0,  0,  0,  0,  0,  0,
+   0,  0,  0,  0,  0,  0,  0,  0,
+   0,  0,  0,  0,  0,  0,  0,  0,
+   0,  0,  0, -1, -1, -1, -1, -1,
+  -1,  0,  0,  0,  0,  0,  0,  0,
+   0,  0,  0,  0,  0,  0,  0,  0,
+   0,  0,  0,  0,  0,  0,  0,  0,
+   0,  0,  0, -1, -1, -1, -1, -1,
 ];
-const kEqualSignIndex = ArrayPrototypeIndexOf(kForgivingBase64AllowedChars,
-                                              0x3D);
+/* eslint-enable no-multi-spaces, indent */
 
 function atob(input) {
   // The implementation here has not been performance optimized in any way and
@@ -1298,16 +1302,17 @@ function atob(input) {
   let equalCharCount = 0;
 
   for (let n = 0; n < input.length; n++) {
-    const index = ArrayPrototypeIndexOf(
-      kForgivingBase64AllowedChars,
-      StringPrototypeCharCodeAt(input, n));
+    const ch = StringPrototypeCharCodeAt(input, n);
+    const val = kForgivingBase64AllowedChars[ch & 0x7f];
 
-    if (index > 4) {
-      // The first 5 elements of `kForgivingBase64AllowedChars` are
-      // ASCII whitespace char codes.
+    if ((ch | val) & ~0x7f) {
+      throw lazyDOMException('Invalid character', 'InvalidCharacterError');
+    }
+
+    if (ch > 0x20) {
       nonAsciiWhitespaceCharCount++;
 
-      if (index === kEqualSignIndex) {
+      if (ch === 0x3d) {
         equalCharCount++;
       } else if (equalCharCount) {
         // The `=` char is only allowed at the end.
@@ -1318,8 +1323,6 @@ function atob(input) {
         // Only one more `=` is permitted after the first equal sign.
         throw lazyDOMException('Invalid character', 'InvalidCharacterError');
       }
-    } else if (index === -1) {
-      throw lazyDOMException('Invalid character', 'InvalidCharacterError');
     }
   }
 

diff --git a/test/parallel/test-btoa-atob.js b/test/parallel/test-btoa-atob.js
@@ -25,6 +25,69 @@ strictEqual(atob([]), '');
 strictEqual(atob({ toString: () => '' }), '');
 strictEqual(atob({ [Symbol.toPrimitive]: () => '' }), '');
 
+const invalidChar = {
+  name: 'InvalidCharacterError'
+};
+
+// Test the entire 16 bit space for invalid characters.
+for (let i = 0; i <= 0xffff; i++) {
+  switch (i) {
+    case 0x09: // \t
+    case 0x0A: // \n
+    case 0x0C: // \f
+    case 0x0D: // \r
+    case 0x20: // ' '
+    case 0x2B: // +
+    case 0x2F: // /
+    case 0x3D: // =
+      continue;
+  }
+
+  // 0-9
+  if (i >= 0x30 && i <= 0x39)
+    continue;
+
+  // A-Z
+  if (i >= 0x41 && i <= 0x5a)
+    continue;
+
+  // a-z
+  if (i >= 0x61 && i <= 0x7a)
+    continue;
+
+  const ch = String.fromCharCode(i);
+
+  throws(() => atob(ch), invalidChar);
+  throws(() => atob('a' + ch), invalidChar);
+  throws(() => atob('aa' + ch), invalidChar);
+  throws(() => atob('aaa' + ch), invalidChar);
+  throws(() => atob(ch + 'a'), invalidChar);
+  throws(() => atob(ch + 'aa'), invalidChar);
+  throws(() => atob(ch + 'aaa'), invalidChar);
+}
+
+throws(() => btoa('abcd\ufeffx'), invalidChar);
+
+const charset =
+  'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/';
+
+function randomString(size) {
+  let str = '';
+
+  for (let i = 0; i < size; i++)
+    str += charset[Math.random() * charset.length | 0];
+
+  while (str.length & 3)
+    str += '=';
+
+  return str;
+}
+
+for (let i = 0; i < 100; i++) {
+  const str = randomString(200);
+  strictEqual(btoa(atob(str)), str);
+}
+
 throws(() => atob(Symbol()), /TypeError/);
 [
   undefined, false, () => {}, {}, [1],