Skip to content

Commit f700074

Browse files
austinkelleherjuanarbol
authored andcommitted
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: #42646 PR-URL: #42662 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Akhil Marsonya <[email protected]>
1 parent db151e1 commit f700074

File tree

2 files changed

+49
-4
lines changed

2 files changed

+49
-4
lines changed

lib/buffer.js

+22-3
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ const {
2626
ArrayFrom,
2727
ArrayIsArray,
2828
ArrayPrototypeForEach,
29-
ArrayPrototypeIncludes,
29+
ArrayPrototypeIndexOf,
3030
MathFloor,
3131
MathMin,
3232
MathTrunc,
@@ -1265,12 +1265,31 @@ function atob(input) {
12651265
if (arguments.length === 0) {
12661266
throw new ERR_MISSING_ARGS('input');
12671267
}
1268+
12681269
input = `${input}`;
1270+
let nonAsciiWhitespaceCharCount = 0;
1271+
12691272
for (let n = 0; n < input.length; n++) {
1270-
if (!ArrayPrototypeIncludes(kForgivingBase64AllowedChars,
1271-
StringPrototypeCharCodeAt(input, n)))
1273+
const index = ArrayPrototypeIndexOf(
1274+
kForgivingBase64AllowedChars,
1275+
StringPrototypeCharCodeAt(input, n));
1276+
1277+
if (index > 4) {
1278+
// The first 5 elements of `kForgivingBase64AllowedChars` are
1279+
// ASCII whitespace char codes.
1280+
nonAsciiWhitespaceCharCount++;
1281+
} else if (index === -1) {
12721282
throw lazyDOMException('Invalid character', 'InvalidCharacterError');
1283+
}
12731284
}
1285+
1286+
// See #3 - https://infra.spec.whatwg.org/#forgiving-base64
1287+
if (nonAsciiWhitespaceCharCount % 4 === 1) {
1288+
throw lazyDOMException(
1289+
'The string to be decoded is not correctly encoded.',
1290+
'InvalidCharacterError');
1291+
}
1292+
12741293
return Buffer.from(input, 'base64').toString('latin1');
12751294
}
12761295

test/parallel/test-btoa-atob.js

+27-1
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
1+
// Flags: --expose-internals
12
'use strict';
23

34
require('../common');
45

56
const { strictEqual, throws } = require('assert');
67
const buffer = require('buffer');
78

9+
const { internalBinding } = require('internal/test/binding');
10+
const { DOMException } = internalBinding('messaging');
11+
812
// Exported on the global object
913
strictEqual(globalThis.atob, buffer.atob);
1014
strictEqual(globalThis.btoa, buffer.btoa);
@@ -14,4 +18,26 @@ throws(() => buffer.atob(), /TypeError/);
1418
throws(() => buffer.btoa(), /TypeError/);
1519

1620
strictEqual(atob(' '), '');
17-
strictEqual(atob(' YW\tJ\njZA=\r= '), 'abcd');
21+
strictEqual(atob(' Y\fW\tJ\njZ A=\r= '), 'abcd');
22+
23+
strictEqual(atob(null), '\x9Eée');
24+
strictEqual(atob(NaN), '5£');
25+
strictEqual(atob(Infinity), '"wâ\x9E+r');
26+
strictEqual(atob(true), '¶»\x9E');
27+
strictEqual(atob(1234), '×mø');
28+
strictEqual(atob([]), '');
29+
strictEqual(atob({ toString: () => '' }), '');
30+
strictEqual(atob({ [Symbol.toPrimitive]: () => '' }), '');
31+
32+
throws(() => atob(Symbol()), /TypeError/);
33+
[
34+
undefined, false, () => {}, {}, [1],
35+
0, 1, 0n, 1n, -Infinity,
36+
'a', 'a\n\n\n', '\ra\r\r', ' a ', '\t\t\ta', 'a\f\f\f', '\ta\r \n\f',
37+
].forEach((value) =>
38+
// See #2 - https://html.spec.whatwg.org/multipage/webappapis.html#dom-atob
39+
throws(() => atob(value), {
40+
constructor: DOMException,
41+
name: 'InvalidCharacterError',
42+
code: 5,
43+
}));

0 commit comments

Comments
 (0)