Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v16.x backport] tools: add avoid-prototype-pollution lint rule #44081

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions doc/contributing/primordials.md
Original file line number Diff line number Diff line change
Expand Up @@ -738,3 +738,27 @@ class SomeClass {
ObjectDefineProperty(SomeClass.prototype, 'readOnlyProperty', kEnumerableProperty);
console.log(new SomeClass().readOnlyProperty); // genuine data
```

### Defining a `Proxy` handler

When defining a `Proxy`, the handler object could be at risk of prototype
pollution when using a plain object literal:

```js
// User-land
Object.prototype.get = () => 'Unrelated user-provided data';

// Core
const objectToProxy = { someProperty: 'genuine value' };

const proxyWithPlainObjectLiteral = new Proxy(objectToProxy, {
has() { return false; },
});
console.log(proxyWithPlainObjectLiteral.someProperty); // Unrelated user-provided data

const proxyWithNullPrototypeObject = new Proxy(objectToProxy, {
__proto__: null,
has() { return false; },
});
console.log(proxyWithNullPrototypeObject.someProperty); // genuine value
```
1 change: 1 addition & 0 deletions lib/.eslintrc.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ rules:
- name: SubtleCrypto
message: Use `const { SubtleCrypto } = require('internal/crypto/webcrypto');` instead of the global.
# Custom rules in tools/eslint-rules
node-core/avoid-prototype-pollution: error
node-core/lowercase-name-for-primitive: error
node-core/non-ascii-character: error
node-core/no-array-destructuring: error
Expand Down
4 changes: 2 additions & 2 deletions lib/_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const {
ObjectKeys,
ObjectSetPrototypeOf,
ReflectApply,
RegExpPrototypeTest,
RegExpPrototypeExec,
String,
StringPrototypeCharCodeAt,
StringPrototypeIncludes,
Expand Down Expand Up @@ -169,7 +169,7 @@ function ClientRequest(input, options, cb) {

if (options.path) {
const path = String(options.path);
if (RegExpPrototypeTest(INVALID_PATH_REGEX, path))
if (RegExpPrototypeExec(INVALID_PATH_REGEX, path) !== null)
throw new ERR_UNESCAPED_CHARACTERS('Request path');
}

Expand Down
6 changes: 3 additions & 3 deletions lib/_http_common.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
const {
MathMin,
Symbol,
RegExpPrototypeTest,
RegExpPrototypeExec,
} = primordials;
const { setImmediate } = require('timers');

Expand Down Expand Up @@ -218,7 +218,7 @@ const tokenRegExp = /^[\^_`a-zA-Z\-0-9!#$%&'*+.|~]+$/;
* See https://tools.ietf.org/html/rfc7230#section-3.2.6
*/
function checkIsHttpToken(val) {
return RegExpPrototypeTest(tokenRegExp, val);
return RegExpPrototypeExec(tokenRegExp, val) !== null;
}

const headerCharRegex = /[^\t\x20-\x7e\x80-\xff]/;
Expand All @@ -229,7 +229,7 @@ const headerCharRegex = /[^\t\x20-\x7e\x80-\xff]/;
* field-vchar = VCHAR / obs-text
*/
function checkInvalidHeaderChar(val) {
return RegExpPrototypeTest(headerCharRegex, val);
return RegExpPrototypeExec(headerCharRegex, val) !== null;
}

function cleanParser(parser) {
Expand Down
6 changes: 3 additions & 3 deletions lib/_http_outgoing.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ const {
ObjectValues,
ObjectPrototypeHasOwnProperty,
ObjectSetPrototypeOf,
RegExpPrototypeTest,
RegExpPrototypeExec,
SafeSet,
StringPrototypeToLowerCase,
Symbol,
Expand Down Expand Up @@ -542,15 +542,15 @@ function matchHeader(self, state, field, value) {
case 'connection':
state.connection = true;
self._removedConnection = false;
if (RegExpPrototypeTest(RE_CONN_CLOSE, value))
if (RegExpPrototypeExec(RE_CONN_CLOSE, value) !== null)
self._last = true;
else
self.shouldKeepAlive = true;
break;
case 'transfer-encoding':
state.te = true;
self._removedTE = false;
if (RegExpPrototypeTest(RE_TE_CHUNKED, value))
if (RegExpPrototypeExec(RE_TE_CHUNKED, value) !== null)
self.chunkedEncoding = true;
break;
case 'content-length':
Expand Down
8 changes: 4 additions & 4 deletions lib/_http_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const {
Error,
ObjectKeys,
ObjectSetPrototypeOf,
RegExpPrototypeTest,
RegExpPrototypeExec,
Symbol,
SymbolFor,
} = primordials;
Expand Down Expand Up @@ -191,8 +191,8 @@ function ServerResponse(req) {
this._expect_continue = false;

if (req.httpVersionMajor < 1 || req.httpVersionMinor < 1) {
this.useChunkedEncodingByDefault = RegExpPrototypeTest(chunkExpression,
req.headers.te);
this.useChunkedEncodingByDefault = RegExpPrototypeExec(chunkExpression,
req.headers.te) !== null;
this.shouldKeepAlive = false;
}

Expand Down Expand Up @@ -957,7 +957,7 @@ function parserOnIncoming(server, socket, state, req, keepAlive) {
} else if (req.headers.expect !== undefined) {
handled = true;

if (RegExpPrototypeTest(continueExpression, req.headers.expect)) {
if (RegExpPrototypeExec(continueExpression, req.headers.expect) !== null) {
res._expect_continue = true;

if (server.listenerCount('checkContinue') > 0) {
Expand Down
32 changes: 16 additions & 16 deletions lib/_tls_common.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const {
ArrayPrototypePush,
JSONParse,
ObjectCreate,
StringPrototypeReplace,
RegExpPrototypeSymbolReplace,
} = primordials;

const {
Expand Down Expand Up @@ -142,21 +142,21 @@ function translatePeerCertificate(c) {
c.infoAccess = ObjectCreate(null);

// XXX: More key validation?
StringPrototypeReplace(info, /([^\n:]*):([^\n]*)(?:\n|$)/g,
(all, key, val) => {
if (val.charCodeAt(0) === 0x22) {
// The translatePeerCertificate function is only
// used on internally created legacy certificate
// objects, and any value that contains a quote
// will always be a valid JSON string literal,
// so this should never throw.
val = JSONParse(val);
}
if (key in c.infoAccess)
ArrayPrototypePush(c.infoAccess[key], val);
else
c.infoAccess[key] = [val];
});
RegExpPrototypeSymbolReplace(/([^\n:]*):([^\n]*)(?:\n|$)/g, info,
(all, key, val) => {
if (val.charCodeAt(0) === 0x22) {
// The translatePeerCertificate function is only
// used on internally created legacy certificate
// objects, and any value that contains a quote
// will always be a valid JSON string literal,
// so this should never throw.
val = JSONParse(val);
}
if (key in c.infoAccess)
ArrayPrototypePush(c.infoAccess[key], val);
else
c.infoAccess[key] = [val];
});
}
return c;
}
Expand Down
17 changes: 9 additions & 8 deletions lib/_tls_wrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,9 @@ const {
ObjectSetPrototypeOf,
ReflectApply,
RegExp,
RegExpPrototypeTest,
StringPrototypeReplace,
RegExpPrototypeExec,
RegExpPrototypeSymbolReplace,
StringPrototypeReplaceAll,
StringPrototypeSlice,
Symbol,
SymbolFor,
Expand Down Expand Up @@ -426,8 +427,8 @@ function onerror(err) {
owner.destroy(err);
} else if (owner._tlsOptions?.isServer &&
owner._rejectUnauthorized &&
RegExpPrototypeTest(/peer did not return a certificate/,
err.message)) {
RegExpPrototypeExec(/peer did not return a certificate/,
err.message) !== null) {
// Ignore server's authorization errors
owner.destroy();
} else {
Expand Down Expand Up @@ -1448,9 +1449,9 @@ Server.prototype.addContext = function(servername, context) {
throw new ERR_TLS_REQUIRED_SERVER_NAME();
}

const re = new RegExp('^' + StringPrototypeReplace(
StringPrototypeReplace(servername, /([.^$+?\-\\[\]{}])/g, '\\$1'),
/\*/g, '[^.]*'
const re = new RegExp('^' + StringPrototypeReplaceAll(
RegExpPrototypeSymbolReplace(/([.^$+?\-\\[\]{}])/g, servername, '\\$1'),
'*', '[^.]*'
) + '$');
ArrayPrototypePush(this._contexts,
[re, tls.createSecureContext(context).context]);
Expand All @@ -1474,7 +1475,7 @@ function SNICallback(servername, callback) {

for (let i = contexts.length - 1; i >= 0; --i) {
const elem = contexts[i];
if (RegExpPrototypeTest(elem[0], servername)) {
if (RegExpPrototypeExec(elem[0], servername) !== null) {
callback(null, elem[1]);
return;
}
Expand Down
15 changes: 8 additions & 7 deletions lib/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ const {
ObjectKeys,
ObjectPrototypeIsPrototypeOf,
ReflectApply,
RegExpPrototypeTest,
RegExpPrototypeExec,
RegExpPrototypeSymbolReplace,
SafeMap,
String,
StringPrototypeCharCodeAt,
Expand Down Expand Up @@ -345,7 +346,7 @@ function getErrMessage(message, fn) {
// Always normalize indentation, otherwise the message could look weird.
if (StringPrototypeIncludes(message, '\n')) {
if (EOL === '\r\n') {
message = StringPrototypeReplace(message, /\r\n/g, '\n');
message = RegExpPrototypeSymbolReplace(/\r\n/g, message, '\n');
}
const frames = StringPrototypeSplit(message, '\n');
message = ArrayPrototypeShift(frames);
Expand Down Expand Up @@ -606,7 +607,7 @@ class Comparison {
if (actual !== undefined &&
typeof actual[key] === 'string' &&
isRegExp(obj[key]) &&
RegExpPrototypeTest(obj[key], actual[key])) {
RegExpPrototypeExec(obj[key], actual[key]) !== null) {
this[key] = actual[key];
} else {
this[key] = obj[key];
Expand Down Expand Up @@ -652,7 +653,7 @@ function expectedException(actual, expected, message, fn) {
// Handle regular expressions.
if (isRegExp(expected)) {
const str = String(actual);
if (RegExpPrototypeTest(expected, str))
if (RegExpPrototypeExec(expected, str) !== null)
return;

if (!message) {
Expand Down Expand Up @@ -687,7 +688,7 @@ function expectedException(actual, expected, message, fn) {
for (const key of keys) {
if (typeof actual[key] === 'string' &&
isRegExp(expected[key]) &&
RegExpPrototypeTest(expected[key], actual[key])) {
RegExpPrototypeExec(expected[key], actual[key]) !== null) {
continue;
}
compareExceptionKey(actual, expected, key, message, keys, fn);
Expand Down Expand Up @@ -851,7 +852,7 @@ function hasMatchingError(actual, expected) {
if (typeof expected !== 'function') {
if (isRegExp(expected)) {
const str = String(actual);
return RegExpPrototypeTest(expected, str);
return RegExpPrototypeExec(expected, str) !== null;
}
throw new ERR_INVALID_ARG_TYPE(
'expected', ['Function', 'RegExp'], expected
Expand Down Expand Up @@ -1000,7 +1001,7 @@ function internalMatch(string, regexp, message, fn) {
}
const match = fn === assert.match;
if (typeof string !== 'string' ||
RegExpPrototypeTest(regexp, string) !== match) {
RegExpPrototypeExec(regexp, string) !== null !== match) {
if (message instanceof Error) {
throw message;
}
Expand Down
6 changes: 3 additions & 3 deletions lib/buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ const {
ObjectDefineProperties,
ObjectDefineProperty,
ObjectSetPrototypeOf,
RegExpPrototypeSymbolReplace,
StringPrototypeCharCodeAt,
StringPrototypeReplace,
StringPrototypeSlice,
StringPrototypeToLowerCase,
StringPrototypeTrim,
Expand Down Expand Up @@ -840,8 +840,8 @@ Buffer.prototype[customInspectSymbol] = function inspect(recurseTimes, ctx) {
const max = INSPECT_MAX_BYTES;
const actualMax = MathMin(max, this.length);
const remaining = this.length - max;
let str = StringPrototypeTrim(StringPrototypeReplace(
this.hexSlice(0, actualMax), /(.{2})/g, '$1 '));
let str = StringPrototypeTrim(RegExpPrototypeSymbolReplace(
/(.{2})/g, this.hexSlice(0, actualMax), '$1 '));
if (remaining > 0)
str += ` ... ${remaining} more byte${remaining > 1 ? 's' : ''}`;
// Inspect special properties as well, if possible.
Expand Down
4 changes: 2 additions & 2 deletions lib/child_process.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ const {
ObjectAssign,
ObjectDefineProperty,
ObjectPrototypeHasOwnProperty,
RegExpPrototypeTest,
RegExpPrototypeExec,
SafeSet,
StringPrototypeSlice,
StringPrototypeToUpperCase,
Expand Down Expand Up @@ -592,7 +592,7 @@ function normalizeSpawnArguments(file, args, options) {
else
file = process.env.comspec || 'cmd.exe';
// '/d /s /c' is used only for cmd.exe.
if (RegExpPrototypeTest(/^(?:.*\\)?cmd(?:\.exe)?$/i, file)) {
if (RegExpPrototypeExec(/^(?:.*\\)?cmd(?:\.exe)?$/i, file) !== null) {
args = ['/d', '/s', '/c', `"${command}"`];
windowsVerbatimArguments = true;
} else {
Expand Down
3 changes: 3 additions & 0 deletions lib/crypto.js
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,7 @@ ObjectDefineProperties(module.exports, {
// Aliases for randomBytes are deprecated.
// The ecosystem needs those to exist for backwards compatibility.
prng: {
__proto__: null,
enumerable: false,
configurable: true,
writable: true,
Expand All @@ -305,6 +306,7 @@ ObjectDefineProperties(module.exports, {
randomBytes
},
pseudoRandomBytes: {
__proto__: null,
enumerable: false,
configurable: true,
writable: true,
Expand All @@ -314,6 +316,7 @@ ObjectDefineProperties(module.exports, {
randomBytes
},
rng: {
__proto__: null,
enumerable: false,
configurable: true,
writable: true,
Expand Down
6 changes: 3 additions & 3 deletions lib/internal/blob.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ const {
PromiseReject,
SafePromisePrototypeFinally,
ReflectConstruct,
RegExpPrototypeExec,
RegExpPrototypeSymbolReplace,
RegExpPrototypeTest,
StringPrototypeToLowerCase,
StringPrototypeSplit,
Symbol,
Expand Down Expand Up @@ -164,7 +164,7 @@ class Blob {
this[kLength] = length;

type = `${type}`;
this[kType] = RegExpPrototypeTest(disallowedTypeCharacters, type) ?
this[kType] = RegExpPrototypeExec(disallowedTypeCharacters, type) !== null ?
'' : StringPrototypeToLowerCase(type);

// eslint-disable-next-line no-constructor-return
Expand Down Expand Up @@ -246,7 +246,7 @@ class Blob {
end |= 0;

contentType = `${contentType}`;
if (RegExpPrototypeTest(disallowedTypeCharacters, contentType)) {
if (RegExpPrototypeExec(disallowedTypeCharacters, contentType) !== null) {
contentType = '';
} else {
contentType = StringPrototypeToLowerCase(contentType);
Expand Down
Loading