buffer: fix atob input validation

This commit fixes a few inconsistencies between Node.js `atob` implementation and the WHATWG spec. Refs: https://infra.spec.whatwg.org/#forgiving-base64-decode Fixes: nodejs/node#42646 PR-URL: nodejs/node#42662 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Akhil Marsonya <[email protected]>
noslate-project · Jan 3, 2023 · e02a18c · e02a18c
1 parent 8b349c8
commit e02a18c
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 4 deletions.
diff --git a/lib/buffer.js b/lib/buffer.js
@@ -26,7 +26,7 @@ const {
   ArrayFrom,
   ArrayIsArray,
   ArrayPrototypeForEach,
-  ArrayPrototypeIncludes,
+  ArrayPrototypeIndexOf,
   MathFloor,
   MathMin,
   MathTrunc,
@@ -1265,12 +1265,31 @@ function atob(input) {
   if (arguments.length === 0) {
     throw new ERR_MISSING_ARGS('input');
   }
+
   input = `${input}`;
+  let nonAsciiWhitespaceCharCount = 0;
+
   for (let n = 0; n < input.length; n++) {
-    if (!ArrayPrototypeIncludes(kForgivingBase64AllowedChars,
-                                StringPrototypeCharCodeAt(input, n)))
+    const index = ArrayPrototypeIndexOf(
+      kForgivingBase64AllowedChars,
+      StringPrototypeCharCodeAt(input, n));
+
+    if (index > 4) {
+      // The first 5 elements of `kForgivingBase64AllowedChars` are
+      // ASCII whitespace char codes.
+      nonAsciiWhitespaceCharCount++;
+    } else if (index === -1) {
       throw lazyDOMException('Invalid character', 'InvalidCharacterError');
+    }
   }
+
+  // See #3 - https://infra.spec.whatwg.org/#forgiving-base64
+  if (nonAsciiWhitespaceCharCount % 4 === 1) {
+    throw lazyDOMException(
+      'The string to be decoded is not correctly encoded.',
+      'InvalidCharacterError');
+  }
+
   return Buffer.from(input, 'base64').toString('latin1');
 }
 

diff --git a/test/parallel/test-btoa-atob.js b/test/parallel/test-btoa-atob.js
@@ -1,10 +1,14 @@
+// Flags: --expose-internals
 'use strict';
 
 require('../common');
 
 const { strictEqual, throws } = require('assert');
 const buffer = require('buffer');
 
+const { internalBinding } = require('internal/test/binding');
+const { DOMException } = internalBinding('messaging');
+
 // Exported on the global object
 strictEqual(globalThis.atob, buffer.atob);
 strictEqual(globalThis.btoa, buffer.btoa);
@@ -14,4 +18,26 @@ throws(() => buffer.atob(), /TypeError/);
 throws(() => buffer.btoa(), /TypeError/);
 
 strictEqual(atob(' '), '');
-strictEqual(atob('  YW\tJ\njZA=\r= '), 'abcd');
+strictEqual(atob('  Y\fW\tJ\njZ A=\r= '), 'abcd');
+
+strictEqual(atob(null), '\x9Eée');
+strictEqual(atob(NaN), '5£');
+strictEqual(atob(Infinity), '"wâ\x9E+r');
+strictEqual(atob(true), '¶»\x9E');
+strictEqual(atob(1234), '×mø');
+strictEqual(atob([]), '');
+strictEqual(atob({ toString: () => '' }), '');
+strictEqual(atob({ [Symbol.toPrimitive]: () => '' }), '');
+
+throws(() => atob(Symbol()), /TypeError/);
+[
+  undefined, false, () => {}, {}, [1],
+  0, 1, 0n, 1n, -Infinity,
+  'a', 'a\n\n\n', '\ra\r\r', '  a ', '\t\t\ta', 'a\f\f\f', '\ta\r \n\f',
+].forEach((value) =>
+  // See #2 - https://html.spec.whatwg.org/multipage/webappapis.html#dom-atob
+  throws(() => atob(value), {
+    constructor: DOMException,
+    name: 'InvalidCharacterError',
+    code: 5,
+  }));