Skip to content

Commit

Permalink
buffer: fix atob input validation
Browse files Browse the repository at this point in the history
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: #42646
PR-URL: #42662
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Akhil Marsonya <akhil.marsonya27@gmail.com>
  • Loading branch information
austinkelleher authored and juanarbol committed Oct 10, 2022
1 parent 4d25566 commit 6bc7635
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 4 deletions.
25 changes: 22 additions & 3 deletions lib/buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const {
ArrayFrom,
ArrayIsArray,
ArrayPrototypeForEach,
ArrayPrototypeIncludes,
ArrayPrototypeIndexOf,
MathFloor,
MathMin,
MathTrunc,
Expand Down Expand Up @@ -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');
}

Expand Down
28 changes: 27 additions & 1 deletion test/parallel/test-btoa-atob.js
Original file line number Diff line number Diff line change
@@ -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);
Expand All @@ -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,
}));

0 comments on commit 6bc7635

Please sign in to comment.