From e4f3c20be260c32d5e5de5ccfb7d9cd3e82ce83e Mon Sep 17 00:00:00 2001 From: RafaelGSS Date: Mon, 5 Jan 2026 18:18:39 -0300 Subject: [PATCH 1/9] permission: add permission check to realpath.native MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: RafaelGSS PR-URL: https://github.com/nodejs-private/node-private/pull/794 Refs: https://hackerone.com/reports/3480841 Reviewed-By: Anna Henningsen Reviewed-By: Marco Ippolito Reviewed-By: Juan José Arboleda Reviewed-By: Matteo Collina CVE-ID: CVE-2026-21715 --- src/node_file.cc | 7 +++++++ test/fixtures/permission/fs-read.js | 14 ++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/src/node_file.cc b/src/node_file.cc index 0fe01e8b08127c..b9e7aba56e9ad1 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -1990,11 +1990,18 @@ static void RealPath(const FunctionCallbackInfo& args) { if (argc > 2) { // realpath(path, encoding, req) FSReqBase* req_wrap_async = GetReqWrap(args, 2); CHECK_NOT_NULL(req_wrap_async); + ASYNC_THROW_IF_INSUFFICIENT_PERMISSIONS( + env, + req_wrap_async, + permission::PermissionScope::kFileSystemRead, + path.ToStringView()); FS_ASYNC_TRACE_BEGIN1( UV_FS_REALPATH, req_wrap_async, "path", TRACE_STR_COPY(*path)) AsyncCall(env, req_wrap_async, args, "realpath", encoding, AfterStringPtr, uv_fs_realpath, *path); } else { // realpath(path, encoding, undefined, ctx) + THROW_IF_INSUFFICIENT_PERMISSIONS( + env, permission::PermissionScope::kFileSystemRead, path.ToStringView()); FSReqWrapSync req_wrap_sync("realpath", *path); FS_SYNC_TRACE_BEGIN(realpath); int err = diff --git a/test/fixtures/permission/fs-read.js b/test/fixtures/permission/fs-read.js index 22f4c4184ae891..fa2bc14e443f99 100644 --- a/test/fixtures/permission/fs-read.js +++ b/test/fixtures/permission/fs-read.js @@ -496,4 +496,18 @@ const regularFile = __filename; fs.lstat(regularFile, (err) => { assert.ifError(err); }); +} + +// fs.realpath.native +{ + fs.realpath.native(blockedFile, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFile), + })); + + // doesNotThrow + fs.realpath.native(regularFile, (err) => { + assert.ifError(err); + }); } \ No newline at end of file From 3a04e0ff25a8c0090b699a606e237e7f52693583 Mon Sep 17 00:00:00 2001 From: RafaelGSS Date: Mon, 5 Jan 2026 20:36:07 -0300 Subject: [PATCH 2/9] permission: include permission check on lib/fs/promises PR-URL: https://github.com/nodejs-private/node-private/pull/795 Refs: https://hackerone.com/reports/3449392 Reviewed-By: Anna Henningsen Reviewed-By: Matteo Collina CVE-ID: CVE-2026-21716 --- lib/internal/fs/promises.js | 16 ++- test/fixtures/permission/fs-read.js | 200 ++++++++++++++++++++++++++ test/fixtures/permission/fs-write.js | 206 ++++++++++++++++++++++++++- 3 files changed, 416 insertions(+), 6 deletions(-) diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index 2f95c4b79e17fd..51c6293e97e292 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -17,6 +17,7 @@ const { Symbol, SymbolAsyncDispose, Uint8Array, + uncurryThis, } = primordials; const { fs: constants } = internalBinding('constants'); @@ -30,6 +31,8 @@ const { const binding = internalBinding('fs'); const { Buffer } = require('buffer'); +const { isBuffer: BufferIsBuffer } = Buffer; +const BufferToString = uncurryThis(Buffer.prototype.toString); const { AbortError, @@ -1018,8 +1021,13 @@ async function fstat(handle, options = { bigint: false }) { } async function lstat(path, options = { bigint: false }) { + path = getValidatedPath(path); + if (permission.isEnabled() && !permission.has('fs.read', path)) { + const resource = pathModule.toNamespacedPath(BufferIsBuffer(path) ? BufferToString(path) : path); + throw new ERR_ACCESS_DENIED('Access to this API has been restricted', 'FileSystemRead', resource); + } const result = await PromisePrototypeThen( - binding.lstat(getValidatedPath(path), options.bigint, kUsePromises), + binding.lstat(path, options.bigint, kUsePromises), undefined, handleErrorFromBinding, ); @@ -1067,6 +1075,9 @@ async function unlink(path) { } async function fchmod(handle, mode) { + if (permission.isEnabled()) { + throw new ERR_ACCESS_DENIED('fchmod API is disabled when Permission Model is enabled.'); + } mode = parseFileMode(mode, 'mode'); return await PromisePrototypeThen( binding.fchmod(handle.fd, mode, kUsePromises), @@ -1107,6 +1118,9 @@ async function lchown(path, uid, gid) { async function fchown(handle, uid, gid) { validateInteger(uid, 'uid', -1, kMaxUserId); validateInteger(gid, 'gid', -1, kMaxUserId); + if (permission.isEnabled()) { + throw new ERR_ACCESS_DENIED('fchown API is disabled when Permission Model is enabled.'); + } return await PromisePrototypeThen( binding.fchown(handle.fd, uid, gid, kUsePromises), undefined, diff --git a/test/fixtures/permission/fs-read.js b/test/fixtures/permission/fs-read.js index fa2bc14e443f99..2e75d57ebd0797 100644 --- a/test/fixtures/permission/fs-read.js +++ b/test/fixtures/permission/fs-read.js @@ -4,6 +4,8 @@ const common = require('../../common'); const assert = require('assert'); const fs = require('fs'); +const fsPromises = require('node:fs/promises'); + const path = require('path'); const { pathToFileURL } = require('url'); @@ -469,6 +471,204 @@ const regularFile = __filename; })); } +// fsPromises.readFile +{ + assert.rejects(async () => { + await fsPromises.readFile(blockedFile); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFile), + })).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.readFile(blockedFileURL); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFile), + })).then(common.mustCall()); +} + +// fsPromises.stat +{ + assert.rejects(async () => { + await fsPromises.stat(blockedFile); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFile), + })).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.stat(blockedFileURL); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFile), + })).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.stat(path.join(blockedFolder, 'anyfile')); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(path.join(blockedFolder, 'anyfile')), + })).then(common.mustCall()); +} + +// fsPromises.access +{ + assert.rejects(async () => { + await fsPromises.access(blockedFile, fs.constants.R_OK); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFile), + })).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.access(blockedFileURL, fs.constants.R_OK); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFile), + })).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.access(path.join(blockedFolder, 'anyfile'), fs.constants.R_OK); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(path.join(blockedFolder, 'anyfile')), + })).then(common.mustCall()); +} + +// fsPromises.copyFile +{ + assert.rejects(async () => { + await fsPromises.copyFile(blockedFile, path.join(blockedFolder, 'any-other-file')); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFile), + })).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.copyFile(blockedFileURL, path.join(blockedFolder, 'any-other-file')); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFile), + })).then(common.mustCall()); +} + +// fsPromises.cp +{ + assert.rejects(async () => { + await fsPromises.cp(blockedFile, path.join(blockedFolder, 'any-other-file')); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFile), + })).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.cp(blockedFileURL, path.join(blockedFolder, 'any-other-file')); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFile), + })).then(common.mustCall()); +} + +// fsPromises.open +{ + assert.rejects(async () => { + await fsPromises.open(blockedFile, 'r'); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFile), + })).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.open(blockedFileURL, 'r'); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFile), + })).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.open(path.join(blockedFolder, 'anyfile'), 'r'); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(path.join(blockedFolder, 'anyfile')), + })).then(common.mustCall()); +} + +// fsPromises.opendir +{ + assert.rejects(async () => { + await fsPromises.opendir(blockedFolder); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFolder), + })).then(common.mustCall()); +} + +// fsPromises.readdir +{ + assert.rejects(async () => { + await fsPromises.readdir(blockedFolder); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFolder), + })).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.readdir(blockedFolder, { recursive: true }); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFolder), + })).then(common.mustCall()); +} + +// fsPromises.rename +{ + assert.rejects(async () => { + await fsPromises.rename(blockedFile, 'newfile'); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFile), + })).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.rename(blockedFileURL, 'newfile'); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFile), + })).then(common.mustCall()); +} + +// fsPromises.lstat +{ + assert.rejects(async () => { + await fsPromises.lstat(blockedFile); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + })).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.lstat(blockedFileURL); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + })).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.lstat(path.join(blockedFolder, 'anyfile')); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + })).then(common.mustCall()); +} + // fs.lstat { assert.throws(() => { diff --git a/test/fixtures/permission/fs-write.js b/test/fixtures/permission/fs-write.js index 27d88911ef8b49..6771485679c1b5 100644 --- a/test/fixtures/permission/fs-write.js +++ b/test/fixtures/permission/fs-write.js @@ -9,6 +9,7 @@ if (!isMainThread) { const assert = require('assert'); const fs = require('fs'); +const fsPromises = require('node:fs/promises'); const path = require('path'); const regularFolder = process.env.ALLOWEDFOLDER; @@ -212,7 +213,7 @@ const relativeProtectedFolder = process.env.RELATIVEBLOCKEDFOLDER; permission: 'FileSystemWrite', })); assert.rejects(async () => { - await fs.promises.mkdtemp(path.join(blockedFolder, 'any-folder')); + await fsPromises.mkdtemp(path.join(blockedFolder, 'any-folder')); }, { code: 'ERR_ACCESS_DENIED', permission: 'FileSystemWrite', @@ -360,7 +361,7 @@ const relativeProtectedFolder = process.env.RELATIVEBLOCKEDFOLDER; permission: 'FileSystemWrite', })); assert.rejects(async () => { - await fs.promises.open(blockedFile, fs.constants.O_RDWR | fs.constants.O_NOFOLLOW); + await fsPromises.open(blockedFile, fs.constants.O_RDWR | fs.constants.O_NOFOLLOW); }, { code: 'ERR_ACCESS_DENIED', permission: 'FileSystemWrite', @@ -399,7 +400,7 @@ const relativeProtectedFolder = process.env.RELATIVEBLOCKEDFOLDER; permission: 'FileSystemWrite', }); assert.rejects(async () => { - await fs.promises.chmod(blockedFile, 0o755); + await fsPromises.chmod(blockedFile, 0o755); }, { code: 'ERR_ACCESS_DENIED', permission: 'FileSystemWrite', @@ -414,7 +415,7 @@ const relativeProtectedFolder = process.env.RELATIVEBLOCKEDFOLDER; permission: 'FileSystemWrite', })); assert.rejects(async () => { - await fs.promises.lchmod(blockedFile, 0o755); + await fsPromises.lchmod(blockedFile, 0o755); }, { code: 'ERR_ACCESS_DENIED', permission: 'FileSystemWrite', @@ -439,7 +440,7 @@ const relativeProtectedFolder = process.env.RELATIVEBLOCKEDFOLDER; permission: 'FileSystemWrite', }); assert.rejects(async () => { - await fs.promises.appendFile(blockedFile, 'new data'); + await fsPromises.appendFile(blockedFile, 'new data'); }, { code: 'ERR_ACCESS_DENIED', permission: 'FileSystemWrite', @@ -628,4 +629,199 @@ const relativeProtectedFolder = process.env.RELATIVEBLOCKEDFOLDER; }, { code: 'ERR_ACCESS_DENIED', }); +} + +// fsPromises.writeFile +{ + assert.rejects(async () => { + await fsPromises.writeFile(blockedFile, 'example'); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(blockedFile), + }).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.writeFile(blockedFileURL, 'example'); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(blockedFile), + }).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.writeFile(path.join(blockedFolder, 'anyfile'), 'example'); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(path.join(blockedFolder, 'anyfile')), + }).then(common.mustCall()); +} + +// fsPromises.utimes +{ + assert.rejects(async () => { + await fsPromises.utimes(blockedFile, new Date(), new Date()); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(blockedFile), + }).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.utimes(blockedFileURL, new Date(), new Date()); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(blockedFile), + }).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.utimes(path.join(blockedFolder, 'anyfile'), new Date(), new Date()); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(path.join(blockedFolder, 'anyfile')), + }).then(common.mustCall()); +} + +// fsPromises.lutimes +{ + assert.rejects(async () => { + await fsPromises.lutimes(blockedFile, new Date(), new Date()); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(blockedFile), + }).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.lutimes(blockedFileURL, new Date(), new Date()); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(blockedFile), + }).then(common.mustCall()); +} + +// fsPromises.mkdir +{ + assert.rejects(async () => { + await fsPromises.mkdir(path.join(blockedFolder, 'any-folder')); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(path.join(blockedFolder, 'any-folder')), + }).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.mkdir(path.join(relativeProtectedFolder, 'any-folder')); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(path.join(relativeProtectedFolder, 'any-folder')), + }).then(common.mustCall()); +} + +// fsPromises.rename +{ + assert.rejects(async () => { + await fsPromises.rename(blockedFile, path.join(blockedFile, 'renamed')); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(blockedFile), + }).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.rename(blockedFileURL, path.join(blockedFile, 'renamed')); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(blockedFile), + }).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.rename(regularFile, path.join(blockedFolder, 'renamed')); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(path.join(blockedFolder, 'renamed')), + }).then(common.mustCall()); +} + +// fsPromises.copyFile +{ + assert.rejects(async () => { + await fsPromises.copyFile(regularFile, path.join(blockedFolder, 'any-file')); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(path.join(blockedFolder, 'any-file')), + }).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.copyFile(regularFile, path.join(relativeProtectedFolder, 'any-file')); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(path.join(relativeProtectedFolder, 'any-file')), + }).then(common.mustCall()); +} + +// fsPromises.cp +{ + assert.rejects(async () => { + await fsPromises.cp(regularFile, path.join(blockedFolder, 'any-file')); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(path.join(blockedFolder, 'any-file')), + }).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.cp(regularFile, path.join(relativeProtectedFolder, 'any-file')); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(path.join(relativeProtectedFolder, 'any-file')), + }).then(common.mustCall()); +} + +// fsPromises.unlink +{ + assert.rejects(async () => { + await fsPromises.unlink(blockedFile); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(blockedFile), + }).then(common.mustCall()); + assert.rejects(async () => { + await fsPromises.unlink(blockedFileURL); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(blockedFile), + }).then(common.mustCall()); +} + +// FileHandle.chmod (fchmod) with read-only fd +{ + assert.rejects(async () => { + // blocked file is allowed to read + const fh = await fsPromises.open(blockedFile, 'r'); + try { + await fh.chmod(0o777); + } finally { + await fh.close(); + } + }, { + code: 'ERR_ACCESS_DENIED', + }).then(common.mustCall()); +} + +// FileHandle.chown (fchown) with read-only fd +{ + assert.rejects(async () => { + // blocked file is allowed to read + const fh = await fsPromises.open(blockedFile, 'r'); + try { + await fh.chown(999, 999); + } finally { + await fh.close(); + } + }, { + code: 'ERR_ACCESS_DENIED', + }).then(common.mustCall()); } \ No newline at end of file From dabb2f5f0c0367bf8e0e116b311b9308111ab4c7 Mon Sep 17 00:00:00 2001 From: RafaelGSS Date: Tue, 10 Feb 2026 10:23:20 -0300 Subject: [PATCH 3/9] src: handle url crash on different url formats Signed-off-by: RafaelGSS PR-URL: https://github.com/nodejs-private/node-private/pull/816 Refs: https://hackerone.com/reports/3546390 CVE-ID: CVE-2026-21712 --- src/node_url.cc | 8 +++++++- test/parallel/test-url-format-whatwg.js | 8 ++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/node_url.cc b/src/node_url.cc index 9b91f83d879ea0..6294cd03667980 100644 --- a/src/node_url.cc +++ b/src/node_url.cc @@ -344,7 +344,13 @@ void BindingData::Format(const FunctionCallbackInfo& args) { // directly want to manipulate the url components without using the respective // setters. therefore we are using ada::url here. auto out = ada::parse(href.ToStringView()); - CHECK(out); + if (!out) { + // If the href cannot be re-parsed (e.g. due to ada parser inconsistencies + // with certain IDN hostnames), return the original href unmodified rather + // than crashing. + args.GetReturnValue().Set(args[0]); + return; + } if (!hash) { out->hash = std::nullopt; diff --git a/test/parallel/test-url-format-whatwg.js b/test/parallel/test-url-format-whatwg.js index f399e0faf1d16a..12594335d6bd67 100644 --- a/test/parallel/test-url-format-whatwg.js +++ b/test/parallel/test-url-format-whatwg.js @@ -147,3 +147,11 @@ test('should format tel: prefix', { skip: !hasIntl }, () => { url.format(new URL('tel:123'), { unicode: true }) ); }); + +// Regression test: url.format should not crash on URLs that ada::url_aggregator +// can parse but ada::url cannot (e.g. special scheme URLs with opaque paths). +test('should not crash on URLs with invalid IDN hostnames', () => { + const u = new URL('ws:xn-\u022B'); + // doesNotThrow + url.format(u, { fragment: false, unicode: false, auth: false, search: false }); +}); From 2e2abc6e895745cc5d04dd203afb8e0083fb6835 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Tue, 17 Feb 2026 14:26:17 +0100 Subject: [PATCH 4/9] tls: wrap SNICallback invocation in try/catch Wrap the owner._SNICallback() invocation in loadSNI() with try/catch to route exceptions through owner.destroy() instead of letting them become uncaught exceptions. This completes the fix from CVE-2026-21637 which added try/catch protection to callALPNCallback, onPskServerCallback, and onPskClientCallback but missed loadSNI(). Without this fix, a remote unauthenticated attacker can crash any Node.js TLS server whose SNICallback may throw on unexpected input by sending a single TLS ClientHello with a crafted server_name value. Fixes: https://hackerone.com/reports/3556769 Refs: https://hackerone.com/reports/3473882 CVE-ID: CVE-2026-21637 PR-URL: https://github.com/nodejs-private/node-private/pull/819 Reviewed-By: Robert Nagy Reviewed-By: Rafael Gonzaga CVE-ID: CVE-2026-21637 --- lib/internal/tls/wrap.js | 30 ++++--- ...ls-psk-alpn-callback-exception-handling.js | 90 +++++++++++++++++++ 2 files changed, 107 insertions(+), 13 deletions(-) diff --git a/lib/internal/tls/wrap.js b/lib/internal/tls/wrap.js index 308180d483908d..d89e501432968a 100644 --- a/lib/internal/tls/wrap.js +++ b/lib/internal/tls/wrap.js @@ -210,23 +210,27 @@ function loadSNI(info) { return requestOCSP(owner, info); let once = false; - owner._SNICallback(servername, (err, context) => { - if (once) - return owner.destroy(new ERR_MULTIPLE_CALLBACK()); - once = true; + try { + owner._SNICallback(servername, (err, context) => { + if (once) + return owner.destroy(new ERR_MULTIPLE_CALLBACK()); + once = true; - if (err) - return owner.destroy(err); + if (err) + return owner.destroy(err); - if (owner._handle === null) - return owner.destroy(new ERR_SOCKET_CLOSED()); + if (owner._handle === null) + return owner.destroy(new ERR_SOCKET_CLOSED()); - // TODO(indutny): eventually disallow raw `SecureContext` - if (context) - owner._handle.sni_context = context.context || context; + // TODO(indutny): eventually disallow raw `SecureContext` + if (context) + owner._handle.sni_context = context.context || context; - requestOCSP(owner, info); - }); + requestOCSP(owner, info); + }); + } catch (err) { + owner.destroy(err); + } } diff --git a/test/parallel/test-tls-psk-alpn-callback-exception-handling.js b/test/parallel/test-tls-psk-alpn-callback-exception-handling.js index e87b68d778035c..881215672ecd0d 100644 --- a/test/parallel/test-tls-psk-alpn-callback-exception-handling.js +++ b/test/parallel/test-tls-psk-alpn-callback-exception-handling.js @@ -332,4 +332,94 @@ describe('TLS callback exception handling', () => { await promise; }); + + // Test 7: SNI callback throwing should emit tlsClientError + it('SNICallback throwing emits tlsClientError', async (t) => { + const server = tls.createServer({ + key: fixtures.readKey('agent2-key.pem'), + cert: fixtures.readKey('agent2-cert.pem'), + SNICallback: (servername, cb) => { + throw new Error('Intentional SNI callback error'); + }, + }); + + t.after(() => server.close()); + + const { promise, resolve, reject } = createTestPromise(); + + server.on('tlsClientError', common.mustCall((err, socket) => { + try { + assert.ok(err instanceof Error); + assert.strictEqual(err.message, 'Intentional SNI callback error'); + socket.destroy(); + resolve(); + } catch (e) { + reject(e); + } + })); + + server.on('secureConnection', () => { + reject(new Error('secureConnection should not fire')); + }); + + await new Promise((res) => server.listen(0, res)); + + const client = tls.connect({ + port: server.address().port, + host: '127.0.0.1', + servername: 'evil.attacker.com', + rejectUnauthorized: false, + }); + + client.on('error', () => {}); + + await promise; + }); + + // Test 8: SNI callback with validation error should emit tlsClientError + it('SNICallback validation error emits tlsClientError', async (t) => { + const server = tls.createServer({ + key: fixtures.readKey('agent2-key.pem'), + cert: fixtures.readKey('agent2-cert.pem'), + SNICallback: (servername, cb) => { + // Simulate common developer pattern: throw on unknown servername + if (servername !== 'expected.example.com') { + throw new Error(`Unknown servername: ${servername}`); + } + cb(null, null); + }, + }); + + t.after(() => server.close()); + + const { promise, resolve, reject } = createTestPromise(); + + server.on('tlsClientError', common.mustCall((err, socket) => { + try { + assert.ok(err instanceof Error); + assert.ok(err.message.includes('Unknown servername')); + socket.destroy(); + resolve(); + } catch (e) { + reject(e); + } + })); + + server.on('secureConnection', () => { + reject(new Error('secureConnection should not fire')); + }); + + await new Promise((res) => server.listen(0, res)); + + const client = tls.connect({ + port: server.address().port, + host: '127.0.0.1', + servername: 'unexpected.domain.com', + rejectUnauthorized: false, + }); + + client.on('error', () => {}); + + await promise; + }); }); From 59c86b1d9377fae95b0523fc16537161dd685f41 Mon Sep 17 00:00:00 2001 From: RafaelGSS Date: Wed, 18 Feb 2026 13:37:49 -0300 Subject: [PATCH 5/9] permission: include permission check to pipe_wrap.cc PR-URL: https://github.com/nodejs-private/node-private/pull/820 Refs: https://hackerone.com/reports/3559715 Reviewed-By: Santiago Gimeno CVE-ID: CVE-2026-21711 --- src/pipe_wrap.cc | 6 +++++- test/parallel/test-permission-net-uds.js | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/pipe_wrap.cc b/src/pipe_wrap.cc index 770f0847aec59f..5100b0fed17455 100644 --- a/src/pipe_wrap.cc +++ b/src/pipe_wrap.cc @@ -162,7 +162,10 @@ PipeWrap::PipeWrap(Environment* env, void PipeWrap::Bind(const FunctionCallbackInfo& args) { PipeWrap* wrap; ASSIGN_OR_RETURN_UNWRAP(&wrap, args.This()); - node::Utf8Value name(args.GetIsolate(), args[0]); + Environment* env = wrap->env(); + node::Utf8Value name(env->isolate(), args[0]); + THROW_IF_INSUFFICIENT_PERMISSIONS( + env, permission::PermissionScope::kNet, name.ToStringView()); int err = uv_pipe_bind2(&wrap->handle_, *name, name.length(), UV_PIPE_NO_TRUNCATE); args.GetReturnValue().Set(err); @@ -193,6 +196,7 @@ void PipeWrap::Listen(const FunctionCallbackInfo& args) { Environment* env = wrap->env(); int backlog; if (!args[0]->Int32Value(env->context()).To(&backlog)) return; + THROW_IF_INSUFFICIENT_PERMISSIONS(env, permission::PermissionScope::kNet, ""); int err = uv_listen( reinterpret_cast(&wrap->handle_), backlog, OnConnection); args.GetReturnValue().Set(err); diff --git a/test/parallel/test-permission-net-uds.js b/test/parallel/test-permission-net-uds.js index 7024c9ff6d3b16..436757e9d8b3cb 100644 --- a/test/parallel/test-permission-net-uds.js +++ b/test/parallel/test-permission-net-uds.js @@ -29,3 +29,21 @@ const tls = require('tls'); client.on('connect', common.mustNotCall('TCP connection should be blocked')); } + +{ + assert.throws(() => { + net.createServer().listen('/tmp/perm-server.sock'); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'Net', + }); +} + +{ + assert.throws(() => { + tls.createServer().listen('/tmp/perm-tls-server.sock'); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'Net', + }); +} From ef5929b5aaae7a14e839e2123f54580d18702df0 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Thu, 19 Feb 2026 15:49:43 +0100 Subject: [PATCH 6/9] http: use null prototype for headersDistinct/trailersDistinct Use { __proto__: null } instead of {} when initializing the headersDistinct and trailersDistinct destination objects. A plain {} inherits from Object.prototype, so when a __proto__ header is received, dest["__proto__"] resolves to Object.prototype (truthy), causing _addHeaderLineDistinct to call .push() on it, which throws an uncaught TypeError and crashes the process. Ref: https://hackerone.com/reports/3560402 PR-URL: https://github.com/nodejs-private/node-private/pull/821 Refs: https://hackerone.com/reports/3560402 Reviewed-By: Marco Ippolito Reviewed-By: Rafael Gonzaga CVE-ID: CVE-2026-21710 --- lib/_http_incoming.js | 4 +-- .../test-http-headers-distinct-proto.js | 36 +++++++++++++++++++ test/parallel/test-http-multiple-headers.js | 16 ++++----- 3 files changed, 46 insertions(+), 10 deletions(-) create mode 100644 test/parallel/test-http-headers-distinct-proto.js diff --git a/lib/_http_incoming.js b/lib/_http_incoming.js index 6cda3a84cee065..04b13358ca717f 100644 --- a/lib/_http_incoming.js +++ b/lib/_http_incoming.js @@ -128,7 +128,7 @@ ObjectDefineProperty(IncomingMessage.prototype, 'headersDistinct', { __proto__: null, get: function() { if (!this[kHeadersDistinct]) { - this[kHeadersDistinct] = {}; + this[kHeadersDistinct] = { __proto__: null }; const src = this.rawHeaders; const dst = this[kHeadersDistinct]; @@ -168,7 +168,7 @@ ObjectDefineProperty(IncomingMessage.prototype, 'trailersDistinct', { __proto__: null, get: function() { if (!this[kTrailersDistinct]) { - this[kTrailersDistinct] = {}; + this[kTrailersDistinct] = { __proto__: null }; const src = this.rawTrailers; const dst = this[kTrailersDistinct]; diff --git a/test/parallel/test-http-headers-distinct-proto.js b/test/parallel/test-http-headers-distinct-proto.js new file mode 100644 index 00000000000000..bd4cb82bd6e6b3 --- /dev/null +++ b/test/parallel/test-http-headers-distinct-proto.js @@ -0,0 +1,36 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const http = require('http'); +const net = require('net'); + +// Regression test: sending a __proto__ header must not crash the server +// when accessing req.headersDistinct or req.trailersDistinct. + +const server = http.createServer(common.mustCall((req, res) => { + const headers = req.headersDistinct; + assert.strictEqual(Object.getPrototypeOf(headers), null); + assert.deepStrictEqual(Object.getOwnPropertyDescriptor(headers, '__proto__').value, ['test']); + res.end(); +})); + +server.listen(0, common.mustCall(() => { + const port = server.address().port; + + const client = net.connect(port, common.mustCall(() => { + client.write( + 'GET / HTTP/1.1\r\n' + + 'Host: localhost\r\n' + + '__proto__: test\r\n' + + 'Connection: close\r\n' + + '\r\n', + ); + })); + + client.on('end', common.mustCall(() => { + server.close(); + })); + + client.resume(); +})); diff --git a/test/parallel/test-http-multiple-headers.js b/test/parallel/test-http-multiple-headers.js index d01bca2fe2173c..75796e1faa9960 100644 --- a/test/parallel/test-http-multiple-headers.js +++ b/test/parallel/test-http-multiple-headers.js @@ -26,13 +26,13 @@ const server = createServer( host, 'transfer-encoding': 'chunked' }); - assert.deepStrictEqual(req.headersDistinct, { + assert.deepStrictEqual(req.headersDistinct, Object.assign({ __proto__: null }, { 'connection': ['close'], 'x-req-a': ['eee', 'fff', 'ggg', 'hhh'], 'x-req-b': ['iii; jjj; kkk; lll'], 'host': [host], - 'transfer-encoding': ['chunked'] - }); + 'transfer-encoding': ['chunked'], + })); req.on('end', common.mustCall(() => { assert.deepStrictEqual(req.rawTrailers, [ @@ -45,7 +45,7 @@ const server = createServer( ); assert.deepStrictEqual( req.trailersDistinct, - { 'x-req-x': ['xxx', 'yyy'], 'x-req-y': ['zzz; www'] } + Object.assign({ __proto__: null }, { 'x-req-x': ['xxx', 'yyy'], 'x-req-y': ['zzz; www'] }) ); res.setHeader('X-Res-a', 'AAA'); @@ -132,14 +132,14 @@ server.listen(0, common.mustCall(() => { 'x-res-d': 'JJJ; KKK; LLL', 'transfer-encoding': 'chunked' }); - assert.deepStrictEqual(res.headersDistinct, { + assert.deepStrictEqual(res.headersDistinct, Object.assign({ __proto__: null }, { 'x-res-a': [ 'AAA', 'BBB', 'CCC' ], 'x-res-b': [ 'DDD; EEE; FFF; GGG' ], 'connection': [ 'close' ], 'x-res-c': [ 'HHH', 'III' ], 'x-res-d': [ 'JJJ; KKK; LLL' ], - 'transfer-encoding': [ 'chunked' ] - }); + 'transfer-encoding': [ 'chunked' ], + })); res.on('end', common.mustCall(() => { assert.deepStrictEqual(res.rawTrailers, [ @@ -153,7 +153,7 @@ server.listen(0, common.mustCall(() => { ); assert.deepStrictEqual( res.trailersDistinct, - { 'x-res-x': ['XXX', 'YYY'], 'x-res-y': ['ZZZ; WWW'] } + Object.assign({ __proto__: null }, { 'x-res-x': ['XXX', 'YYY'], 'x-res-y': ['ZZZ; WWW'] }) ); server.close(); })); From b36d5a3d94dc3a1ddbcc35d51ce19d1fa61e8e47 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Fri, 20 Feb 2026 12:32:14 +0100 Subject: [PATCH 7/9] crypto: use timing-safe comparison in Web Cryptography HMAC and KMAC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use `CRYPTO_memcmp` instead of `memcmp` in `HMAC` and `KMAC` Web Cryptography algorithm implementations. Ref: https://hackerone.com/reports/3533945 PR-URL: https://github.com/nodejs-private/node-private/pull/822 Refs: https://hackerone.com/reports/3533945 Reviewed-By: Anna Henningsen Reviewed-By: Rafael Gonzaga Reviewed-By: Сковорода Никита Андреевич CVE-ID: CVE-2026-21713 --- src/crypto/crypto_hmac.cc | 3 ++- src/crypto/crypto_kmac.cc | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/crypto/crypto_hmac.cc b/src/crypto/crypto_hmac.cc index dadb0fc8017e46..88a512d7550200 100644 --- a/src/crypto/crypto_hmac.cc +++ b/src/crypto/crypto_hmac.cc @@ -270,7 +270,8 @@ MaybeLocal HmacTraits::EncodeOutput(Environment* env, return Boolean::New( env->isolate(), out->size() > 0 && out->size() == params.signature.size() && - memcmp(out->data(), params.signature.data(), out->size()) == 0); + CRYPTO_memcmp( + out->data(), params.signature.data(), out->size()) == 0); } UNREACHABLE(); } diff --git a/src/crypto/crypto_kmac.cc b/src/crypto/crypto_kmac.cc index 7dafa9f6d14b1b..c862a20f410d9d 100644 --- a/src/crypto/crypto_kmac.cc +++ b/src/crypto/crypto_kmac.cc @@ -202,7 +202,8 @@ MaybeLocal KmacTraits::EncodeOutput(Environment* env, return Boolean::New( env->isolate(), out->size() > 0 && out->size() == params.signature.size() && - memcmp(out->data(), params.signature.data(), out->size()) == 0); + CRYPTO_memcmp( + out->data(), params.signature.data(), out->size()) == 0); } UNREACHABLE(); } From 82615369d4d9aab2d841d4d790263d2c6336f2e4 Mon Sep 17 00:00:00 2001 From: RafaelGSS Date: Wed, 11 Mar 2026 11:22:23 -0300 Subject: [PATCH 8/9] src: handle NGHTTP2_ERR_FLOW_CONTROL error code Refs: https://hackerone.com/reports/3531737 PR-URL: https://github.com/nodejs-private/node-private/pull/832 CVE-ID: CVE-2026-21714 --- src/node_http2.cc | 6 ++ .../test-http2-window-update-overflow.js | 84 +++++++++++++++++++ 2 files changed, 90 insertions(+) create mode 100644 test/parallel/test-http2-window-update-overflow.js diff --git a/src/node_http2.cc b/src/node_http2.cc index e6e7329dc78216..084a4773673e5b 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -1154,8 +1154,14 @@ int Http2Session::OnInvalidFrame(nghttp2_session* handle, // The GOAWAY frame includes an error code that indicates the type of error" // The GOAWAY frame is already sent by nghttp2. We emit the error // to liberate the Http2Session to destroy. + // + // ERR_FLOW_CONTROL: A WINDOW_UPDATE on stream 0 pushed the connection-level + // flow control window past 2^31-1. nghttp2 sends GOAWAY internally but + // without propagating this error the Http2Session would never be destroyed, + // causing a memory leak. if (nghttp2_is_fatal(lib_error_code) || lib_error_code == NGHTTP2_ERR_STREAM_CLOSED || + lib_error_code == NGHTTP2_ERR_FLOW_CONTROL || lib_error_code == NGHTTP2_ERR_PROTO) { Environment* env = session->env(); Isolate* isolate = env->isolate(); diff --git a/test/parallel/test-http2-window-update-overflow.js b/test/parallel/test-http2-window-update-overflow.js new file mode 100644 index 00000000000000..41488af9b08fcf --- /dev/null +++ b/test/parallel/test-http2-window-update-overflow.js @@ -0,0 +1,84 @@ +'use strict'; + +const common = require('../common'); + +if (!common.hasCrypto) + common.skip('missing crypto'); + +const http2 = require('http2'); +const net = require('net'); + +// Regression test: a connection-level WINDOW_UPDATE that causes the flow +// control window to exceed 2^31-1 must destroy the Http2Session (not leak it). +// +// nghttp2 responds with GOAWAY(FLOW_CONTROL_ERROR) internally but previously +// Node's OnInvalidFrame callback only propagated errors for +// NGHTTP2_ERR_STREAM_CLOSED and NGHTTP2_ERR_PROTO. The missing +// NGHTTP2_ERR_FLOW_CONTROL case left the session unreachable after the GOAWAY, +// causing a memory leak. + +const server = http2.createServer(); + +server.on('session', common.mustCall((session) => { + session.on('error', common.mustCall()); + session.on('close', common.mustCall(() => server.close())); +})); + +server.listen(0, common.mustCall(() => { + const conn = net.connect({ + port: server.address().port, + allowHalfOpen: true, + }); + + // HTTP/2 client connection preface. + conn.write('PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n'); + + // Empty SETTINGS frame (9-byte header, 0-byte payload). + const settingsFrame = Buffer.alloc(9); + settingsFrame[3] = 0x04; // type: SETTINGS + conn.write(settingsFrame); + + let inbuf = Buffer.alloc(0); + let state = 'settingsHeader'; + let settingsFrameLength; + + conn.on('data', (chunk) => { + inbuf = Buffer.concat([inbuf, chunk]); + + switch (state) { + case 'settingsHeader': + if (inbuf.length < 9) return; + settingsFrameLength = inbuf.readUIntBE(0, 3); + inbuf = inbuf.slice(9); + state = 'readingSettings'; + // Fallthrough + case 'readingSettings': { + if (inbuf.length < settingsFrameLength) return; + inbuf = inbuf.slice(settingsFrameLength); + state = 'done'; + + // ACK the server SETTINGS. + const ack = Buffer.alloc(9); + ack[3] = 0x04; // type: SETTINGS + ack[4] = 0x01; // flag: ACK + conn.write(ack); + + // WINDOW_UPDATE on stream 0 (connection level) with increment 2^31-1. + // Default connection window is 65535, so the new total would be + // 65535 + 2147483647 = 2147549182 > 2^31-1, triggering + // NGHTTP2_ERR_FLOW_CONTROL inside nghttp2. + const windowUpdate = Buffer.alloc(13); + windowUpdate.writeUIntBE(4, 0, 3); // length = 4 + windowUpdate[3] = 0x08; // type: WINDOW_UPDATE + windowUpdate[4] = 0x00; // flags: none + windowUpdate.writeUIntBE(0, 5, 4); // stream id: 0 + windowUpdate.writeUIntBE(0x7FFFFFFF, 9, 4); // increment: 2^31-1 + conn.write(windowUpdate); + } + } + }); + + // The server must close the connection after sending GOAWAY. + conn.on('end', common.mustCall(() => conn.end())); + conn.on('close', common.mustCall()); +})); From 0d7e4b1d4b0220b68bc2c5c2b973deff1664824b Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 29 Jan 2026 03:30:37 +0100 Subject: [PATCH 9/9] build,test: test array index hash collision This enables v8_enable_seeded_array_index_hash and add a test for it. Fixes: https://hackerone.com/reports/3511792 deps: V8: backport 0a8b1cdcc8b2 Original commit message: implement rapidhash secret generation Bug: 409717082 Change-Id: I471f33d66de32002f744aeba534c1d34f71e27d2 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6733490 Reviewed-by: Leszek Swirski Commit-Queue: snek Cr-Commit-Position: refs/heads/main@{#101499} Refs: https://github.com/v8/v8/commit/0a8b1cdcc8b243c62cf045fa8beb50600e11758a Co-authored-by: Joyee Cheung deps: V8: backport 185f0fe09b72 Original commit message: [numbers] Refactor HashSeed as a lightweight view over ByteArray Instead of copying the seed and secrets into a struct with value fields, HashSeed now stores a pointer pointing either into the read-only ByteArray, or the static default seed for off-heap HashSeed::Default() calls. The underlying storage is always 8-byte aligned so we can cast it directly into a struct. Change-Id: I5896a7f2ae24296eb4c80b757a5d90ac70a34866 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7609720 Reviewed-by: Leszek Swirski Commit-Queue: Joyee Cheung Cr-Commit-Position: refs/heads/main@{#105531} Refs: https://github.com/v8/v8/commit/185f0fe09b72fb869fdcf9a89f40ff2295436bca Co-authored-by: Joyee Cheung deps: V8: backport 1361b2a49d02 Original commit message: [strings] improve array index hash distribution Previously, the hashes stored in a Name's raw_hash_field for decimal numeric strings (potential array indices) consist of the literal integer value along with the length of the string. This means consecutive numeric strings can have consecutive hash values, which can lead to O(n^2) probing for insertion in the worst case when e.g. a non-numeric string happen to land in the these buckets. This patch adds a build-time flag v8_enable_seeded_array_index_hash that scrambles the 24-bit array-index value stored in a Name's raw_hash_field to improve the distribution. x ^= x >> kShift; x = (x * m1) & kMask; // round 1 x ^= x >> kShift; x = (x * m2) & kMask; // round 2 x ^= x >> kShift; // finalize To decode, apply the same steps with the modular inverses of m1 and m2 in reverse order. x ^= x >> kShift; x = (x * m2_inv) & kMask; // round 1 x ^= x >> kShift; x = (x * m1_inv) & kMask; // round 2 x ^= x >> kShift; // finalize where kShift = kArrayIndexValueBits / 2, kMask = kArrayIndexValueMask, m1, m2 (both odd) are the lower bits of the rapidhash secrets, m1_inv, m2_inv (modular inverses) are precomputed modular inverse of m1 and m2. The pre-computed values are appended to the hash_seed ByteArray in ReadOnlyRoots and accessed in generated code to reduce overhead. In call sites that don't already have access to the seeds, we read them from the current isolate group/isolate's read only roots. To consolidate the code that encode/decode these hashes, this patch adds MakeArrayIndexHash/DecodeArrayIndexFromHashField in C++ and CSA that perform seeding/unseeding if enabled, and updates places where encoding/decoding of array index is needed to use them. Bug: 477515021 Change-Id: I350afe511951a54c4378396538152cc56565fd55 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7564330 Reviewed-by: Leszek Swirski Commit-Queue: Joyee Cheung Cr-Commit-Position: refs/heads/main@{#105596} Refs: https://github.com/v8/v8/commit/1361b2a49d020a718dc5495713eae0fa67d697b9 Co-authored-by: Joyee Cheung deps: V8: cherry-pick aac14dd95e5b Original commit message: [string] add 3rd round to seeded array index hash Since we already have 3 derived secrets, and arithmetics are relatively cheap, add a 3rd round to the xorshift-multiply seeding scheme. This brings the bias from ~3.4 to ~0.4. Bug: 477515021 Change-Id: I1ef48954bcee8768d8c90db06ac8adb02f06cebf Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7655117 Reviewed-by: Chengzhong Wu Commit-Queue: Joyee Cheung Reviewed-by: Leszek Swirski Cr-Commit-Position: refs/heads/main@{#105824} Refs: https://github.com/v8/v8/commit/aac14dd95e5be0d487eba6bcdaf9cef4f8bd806c PR-URL: https://github.com/nodejs-private/node-private/pull/834 CVE-ID: CVE-2026-21717 deps: V8: backport 185f0fe09b72 Original commit message: [numbers] Refactor HashSeed as a lightweight view over ByteArray Instead of copying the seed and secrets into a struct with value fields, HashSeed now stores a pointer pointing either into the read-only ByteArray, or the static default seed for off-heap HashSeed::Default() calls. The underlying storage is always 8-byte aligned so we can cast it directly into a struct. Change-Id: I5896a7f2ae24296eb4c80b757a5d90ac70a34866 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7609720 Reviewed-by: Leszek Swirski Commit-Queue: Joyee Cheung Cr-Commit-Position: refs/heads/main@{#105531} Refs: https://github.com/v8/v8/commit/185f0fe09b72fb869fdcf9a89f40ff2295436bca Co-authored-by: Joyee Cheung deps: V8: backport 1361b2a49d02 Original commit message: [strings] improve array index hash distribution Previously, the hashes stored in a Name's raw_hash_field for decimal numeric strings (potential array indices) consist of the literal integer value along with the length of the string. This means consecutive numeric strings can have consecutive hash values, which can lead to O(n^2) probing for insertion in the worst case when e.g. a non-numeric string happen to land in the these buckets. This patch adds a build-time flag v8_enable_seeded_array_index_hash that scrambles the 24-bit array-index value stored in a Name's raw_hash_field to improve the distribution. x ^= x >> kShift; x = (x * m1) & kMask; // round 1 x ^= x >> kShift; x = (x * m2) & kMask; // round 2 x ^= x >> kShift; // finalize To decode, apply the same steps with the modular inverses of m1 and m2 in reverse order. x ^= x >> kShift; x = (x * m2_inv) & kMask; // round 1 x ^= x >> kShift; x = (x * m1_inv) & kMask; // round 2 x ^= x >> kShift; // finalize where kShift = kArrayIndexValueBits / 2, kMask = kArrayIndexValueMask, m1, m2 (both odd) are the lower bits of the rapidhash secrets, m1_inv, m2_inv (modular inverses) are precomputed modular inverse of m1 and m2. The pre-computed values are appended to the hash_seed ByteArray in ReadOnlyRoots and accessed in generated code to reduce overhead. In call sites that don't already have access to the seeds, we read them from the current isolate group/isolate's read only roots. To consolidate the code that encode/decode these hashes, this patch adds MakeArrayIndexHash/DecodeArrayIndexFromHashField in C++ and CSA that perform seeding/unseeding if enabled, and updates places where encoding/decoding of array index is needed to use them. Bug: 477515021 Change-Id: I350afe511951a54c4378396538152cc56565fd55 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7564330 Reviewed-by: Leszek Swirski Commit-Queue: Joyee Cheung Cr-Commit-Position: refs/heads/main@{#105596} Refs: https://github.com/v8/v8/commit/1361b2a49d020a718dc5495713eae0fa67d697b9 Co-authored-by: Joyee Cheung deps: V8: cherry-pick aac14dd95e5b Original commit message: [string] add 3rd round to seeded array index hash Since we already have 3 derived secrets, and arithmetics are relatively cheap, add a 3rd round to the xorshift-multiply seeding scheme. This brings the bias from ~3.4 to ~0.4. Bug: 477515021 Change-Id: I1ef48954bcee8768d8c90db06ac8adb02f06cebf Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7655117 Reviewed-by: Chengzhong Wu Commit-Queue: Joyee Cheung Reviewed-by: Leszek Swirski Cr-Commit-Position: refs/heads/main@{#105824} Refs: https://github.com/v8/v8/commit/aac14dd95e5be0d487eba6bcdaf9cef4f8bd806c --- common.gypi | 2 +- deps/v8/BUILD.bazel | 7 ++ deps/v8/BUILD.gn | 7 ++ deps/v8/src/DEPS | 2 +- deps/v8/src/ast/ast-value-factory.cc | 3 +- deps/v8/src/builtins/number.tq | 4 +- deps/v8/src/builtins/wasm.tq | 4 +- deps/v8/src/codegen/code-stub-assembler.cc | 68 ++++++++++- deps/v8/src/codegen/code-stub-assembler.h | 6 + deps/v8/src/heap/factory-base.cc | 15 +-- deps/v8/src/heap/factory-base.h | 9 +- deps/v8/src/heap/factory.cc | 3 +- deps/v8/src/heap/heap.cc | 26 ---- deps/v8/src/heap/heap.h | 3 - deps/v8/src/heap/setup-heap-internal.cc | 8 +- deps/v8/src/numbers/hash-seed-inl.h | 34 +++--- deps/v8/src/numbers/hash-seed.cc | 114 ++++++++++++++++++ deps/v8/src/numbers/hash-seed.h | 82 +++++++++++-- deps/v8/src/objects/fixed-array-inl.h | 11 +- deps/v8/src/objects/fixed-array.h | 6 +- deps/v8/src/objects/name.h | 14 ++- deps/v8/src/objects/name.tq | 43 ++++++- deps/v8/src/objects/string-inl.h | 6 +- deps/v8/src/objects/string-table.cc | 3 +- deps/v8/src/objects/string.cc | 6 +- .../v8/src/snapshot/read-only-deserializer.cc | 3 +- .../src/snapshot/shared-heap-deserializer.cc | 2 +- deps/v8/src/strings/string-hasher-inl.h | 75 +++++++++++- deps/v8/src/strings/string-hasher.h | 53 +++++++- deps/v8/src/torque/torque-parser.cc | 5 + deps/v8/test/cctest/test-strings.cc | 67 +++++++--- test/fixtures/array-hash-collision.js | 27 +++++ test/pummel/test-array-hash-collision.js | 27 +++++ tools/v8_gypfiles/features.gypi | 6 + 34 files changed, 623 insertions(+), 128 deletions(-) create mode 100644 deps/v8/src/numbers/hash-seed.cc create mode 100644 test/fixtures/array-hash-collision.js create mode 100644 test/pummel/test-array-hash-collision.js diff --git a/common.gypi b/common.gypi index c58aa7fd89305d..f7c219fc8f167d 100644 --- a/common.gypi +++ b/common.gypi @@ -38,7 +38,7 @@ # Reset this number to 0 on major V8 upgrades. # Increment by one for each non-official patch applied to deps/v8. - 'v8_embedder_string': '-node.13', + 'v8_embedder_string': '-node.16', ##### V8 defaults for Node.js ##### diff --git a/deps/v8/BUILD.bazel b/deps/v8/BUILD.bazel index d49da552c42612..91da3cadbb526a 100644 --- a/deps/v8/BUILD.bazel +++ b/deps/v8/BUILD.bazel @@ -233,6 +233,11 @@ v8_flag( default = False, ) +v8_flag( + name = "v8_enable_seeded_array_index_hash", + default = False, +) + selects.config_setting_group( name = "enable_drumbrake_x64", match_all = [ @@ -505,6 +510,7 @@ v8_config( "v8_enable_webassembly": "V8_ENABLE_WEBASSEMBLY", "v8_enable_drumbrake": "V8_ENABLE_DRUMBRAKE", "v8_enable_drumbrake_tracing": "V8_ENABLE_DRUMBRAKE_TRACING", + "v8_enable_seeded_array_index_hash": "V8_ENABLE_SEEDED_ARRAY_INDEX_HASH", "v8_jitless": "V8_JITLESS", "v8_enable_vtunejit": "ENABLE_VTUNE_JIT_INTERFACE", }, @@ -1993,6 +1999,7 @@ filegroup( "src/numbers/conversions.h", "src/numbers/conversions-inl.h", "src/numbers/hash-seed.h", + "src/numbers/hash-seed.cc", "src/numbers/hash-seed-inl.h", "src/numbers/ieee754.cc", "src/numbers/ieee754.h", diff --git a/deps/v8/BUILD.gn b/deps/v8/BUILD.gn index 3a51ee81fadd13..6432f7342e26a5 100644 --- a/deps/v8/BUILD.gn +++ b/deps/v8/BUILD.gn @@ -489,6 +489,9 @@ declare_args() { # Use a hard-coded secret value when hashing. v8_use_default_hasher_secret = true + + # Enable seeded array index hash. + v8_enable_seeded_array_index_hash = false } # Derived defaults. @@ -1200,6 +1203,9 @@ config("features") { if (v8_enable_lite_mode) { defines += [ "V8_LITE_MODE" ] } + if (v8_enable_seeded_array_index_hash) { + defines += [ "V8_ENABLE_SEEDED_ARRAY_INDEX_HASH" ] + } if (v8_enable_gdbjit) { defines += [ "ENABLE_GDB_JIT_INTERFACE" ] } @@ -5778,6 +5784,7 @@ v8_source_set("v8_base_without_compiler") { "src/logging/runtime-call-stats.cc", "src/logging/tracing-flags.cc", "src/numbers/conversions.cc", + "src/numbers/hash-seed.cc", "src/numbers/ieee754.cc", "src/numbers/math-random.cc", "src/objects/abstract-code.cc", diff --git a/deps/v8/src/DEPS b/deps/v8/src/DEPS index d6da189c53496c..2dd9e3a69c3ef8 100644 --- a/deps/v8/src/DEPS +++ b/deps/v8/src/DEPS @@ -137,7 +137,7 @@ specific_include_rules = { "heap\.cc": [ "+third_party/rapidhash-v8/secret.h", ], - "hash-seed-inl\.h": [ + "hash-seed\.cc": [ "+third_party/rapidhash-v8/secret.h", ], } diff --git a/deps/v8/src/ast/ast-value-factory.cc b/deps/v8/src/ast/ast-value-factory.cc index 2de885e9eb47a5..277d21029a82c0 100644 --- a/deps/v8/src/ast/ast-value-factory.cc +++ b/deps/v8/src/ast/ast-value-factory.cc @@ -83,7 +83,8 @@ bool AstRawString::AsArrayIndex(uint32_t* index) const { // can't be convertible to an array index. if (!IsIntegerIndex()) return false; if (length() <= Name::kMaxCachedArrayIndexLength) { - *index = Name::ArrayIndexValueBits::decode(raw_hash_field_); + *index = StringHasher::DecodeArrayIndexFromHashField( + raw_hash_field_, HashSeed(GetReadOnlyRoots())); return true; } // Might be an index, but too big to cache it. Do the slow conversion. This diff --git a/deps/v8/src/builtins/number.tq b/deps/v8/src/builtins/number.tq index bc00018ecb3d44..b68267a096b926 100644 --- a/deps/v8/src/builtins/number.tq +++ b/deps/v8/src/builtins/number.tq @@ -300,7 +300,7 @@ transitioning javascript builtin NumberParseFloat( const hash: NameHash = s.raw_hash_field; if (IsIntegerIndex(hash) && hash.array_index_length < kMaxCachedArrayIndexLength) { - const arrayIndex: uint32 = hash.array_index_value; + const arrayIndex: uint32 = DecodeArrayIndexFromHashField(hash); return SmiFromUint32(arrayIndex); } // Fall back to the runtime to convert string to a number. @@ -351,7 +351,7 @@ transitioning builtin ParseInt( const hash: NameHash = s.raw_hash_field; if (IsIntegerIndex(hash) && hash.array_index_length < kMaxCachedArrayIndexLength) { - const arrayIndex: uint32 = hash.array_index_value; + const arrayIndex: uint32 = DecodeArrayIndexFromHashField(hash); return SmiFromUint32(arrayIndex); } // Fall back to the runtime. diff --git a/deps/v8/src/builtins/wasm.tq b/deps/v8/src/builtins/wasm.tq index fca963da6699fd..13473a571f2135 100644 --- a/deps/v8/src/builtins/wasm.tq +++ b/deps/v8/src/builtins/wasm.tq @@ -1583,8 +1583,8 @@ builtin WasmStringToDouble(s: String): float64 { const hash: NameHash = s.raw_hash_field; if (IsIntegerIndex(hash) && hash.array_index_length < kMaxCachedArrayIndexLength) { - const arrayIndex: int32 = Signed(hash.array_index_value); - return Convert(arrayIndex); + const arrayIndex: uint32 = DecodeArrayIndexFromHashField(hash); + return Convert(Signed(arrayIndex)); } return StringToFloat64(Flatten(s)); } diff --git a/deps/v8/src/codegen/code-stub-assembler.cc b/deps/v8/src/codegen/code-stub-assembler.cc index 6a533de4906298..1dd38057fb1850 100644 --- a/deps/v8/src/codegen/code-stub-assembler.cc +++ b/deps/v8/src/codegen/code-stub-assembler.cc @@ -2682,6 +2682,66 @@ TNode CodeStubAssembler::LoadJSReceiverIdentityHash( return var_hash.value(); } +#ifdef V8_ENABLE_SEEDED_ARRAY_INDEX_HASH +// Mirror C++ StringHasher::SeedArrayIndexValue. +TNode CodeStubAssembler::SeedArrayIndexValue(TNode value) { + // Load m1, m2 and m3 from the hash seed byte array. In the compiled code + // these will always come from the read-only roots. + TNode hash_seed = CAST(LoadRoot(RootIndex::kHashSeed)); + intptr_t base_offset = OFFSET_OF_DATA_START(ByteArray) - kHeapObjectTag; + TNode m1 = Load( + hash_seed, IntPtrConstant(base_offset + HashSeed::kDerivedM1Offset)); + TNode m2 = Load( + hash_seed, IntPtrConstant(base_offset + HashSeed::kDerivedM2Offset)); + TNode m3 = Load( + hash_seed, IntPtrConstant(base_offset + HashSeed::kDerivedM3Offset)); + + TNode x = value; + // 3-round xorshift-multiply. + x = Word32Xor(x, Word32Shr(x, Uint32Constant(Name::kArrayIndexHashShift))); + x = Word32And(Uint32Mul(Unsigned(x), m1), + Uint32Constant(Name::kArrayIndexValueMask)); + x = Word32Xor(x, Word32Shr(x, Uint32Constant(Name::kArrayIndexHashShift))); + x = Word32And(Uint32Mul(Unsigned(x), m2), + Uint32Constant(Name::kArrayIndexValueMask)); + x = Word32Xor(x, Word32Shr(x, Uint32Constant(Name::kArrayIndexHashShift))); + x = Word32And(Uint32Mul(Unsigned(x), m3), + Uint32Constant(Name::kArrayIndexValueMask)); + x = Word32Xor(x, Word32Shr(x, Uint32Constant(Name::kArrayIndexHashShift))); + + return Unsigned(x); +} + +// Mirror C++ StringHasher::UnseedArrayIndexValue. +TNode CodeStubAssembler::UnseedArrayIndexValue(TNode value) { + // Load m1_inv, m2_inv and m3_inv from the hash seed byte array. In the + // compiled code these will always come from the read-only roots. + TNode hash_seed = CAST(LoadRoot(RootIndex::kHashSeed)); + intptr_t base_offset = OFFSET_OF_DATA_START(ByteArray) - kHeapObjectTag; + TNode m1_inv = Load( + hash_seed, IntPtrConstant(base_offset + HashSeed::kDerivedM1InvOffset)); + TNode m2_inv = Load( + hash_seed, IntPtrConstant(base_offset + HashSeed::kDerivedM2InvOffset)); + TNode m3_inv = Load( + hash_seed, IntPtrConstant(base_offset + HashSeed::kDerivedM3InvOffset)); + + TNode x = value; + // 3-round xorshift-multiply (inverse). + // Xorshift is an involution when kShift is at least half of the value width. + x = Word32Xor(x, Word32Shr(x, Uint32Constant(Name::kArrayIndexHashShift))); + x = Word32And(Uint32Mul(Unsigned(x), m3_inv), + Uint32Constant(Name::kArrayIndexValueMask)); + x = Word32Xor(x, Word32Shr(x, Uint32Constant(Name::kArrayIndexHashShift))); + x = Word32And(Uint32Mul(Unsigned(x), m2_inv), + Uint32Constant(Name::kArrayIndexValueMask)); + x = Word32Xor(x, Word32Shr(x, Uint32Constant(Name::kArrayIndexHashShift))); + x = Word32And(Uint32Mul(Unsigned(x), m1_inv), + Uint32Constant(Name::kArrayIndexValueMask)); + x = Word32Xor(x, Word32Shr(x, Uint32Constant(Name::kArrayIndexHashShift))); + return Unsigned(x); +} +#endif // V8_ENABLE_SEEDED_ARRAY_INDEX_HASH + TNode CodeStubAssembler::LoadNameHashAssumeComputed(TNode name) { TNode hash_field = LoadNameRawHash(name); CSA_DCHECK(this, IsClearWord32(hash_field, Name::kHashNotComputedMask)); @@ -9322,8 +9382,7 @@ TNode CodeStubAssembler::StringToNumber(TNode input) { GotoIf(IsSetWord32(raw_hash_field, Name::kDoesNotContainCachedArrayIndexMask), &runtime); - var_result = SmiTag(Signed( - DecodeWordFromWord32(raw_hash_field))); + var_result = SmiFromUint32(DecodeArrayIndexFromHashField(raw_hash_field)); Goto(&end); BIND(&runtime); @@ -10413,9 +10472,8 @@ void CodeStubAssembler::TryToName(TNode key, Label* if_keyisindex, BIND(&if_has_cached_index); { - TNode index = - Signed(DecodeWordFromWord32( - raw_hash_field)); + TNode index = Signed(ChangeUint32ToWord( + DecodeArrayIndexFromHashField(raw_hash_field))); CSA_DCHECK(this, IntPtrLessThan(index, IntPtrConstant(INT_MAX))); *var_index = index; Goto(if_keyisindex); diff --git a/deps/v8/src/codegen/code-stub-assembler.h b/deps/v8/src/codegen/code-stub-assembler.h index 00e7b9ea56fdb5..e6164f8a53cf00 100644 --- a/deps/v8/src/codegen/code-stub-assembler.h +++ b/deps/v8/src/codegen/code-stub-assembler.h @@ -4780,6 +4780,12 @@ class V8_EXPORT_PRIVATE CodeStubAssembler return WordEqual(WordAnd(flags, IntPtrConstant(mask)), IntPtrConstant(0)); } +#ifdef V8_ENABLE_SEEDED_ARRAY_INDEX_HASH + // Mirror C++ StringHasher::SeedArrayIndexValue and UnseedArrayIndexValue. + TNode SeedArrayIndexValue(TNode value); + TNode UnseedArrayIndexValue(TNode value); +#endif // V8_ENABLE_SEEDED_ARRAY_INDEX_HASH + private: friend class CodeStubArguments; diff --git a/deps/v8/src/heap/factory-base.cc b/deps/v8/src/heap/factory-base.cc index c3e897021ec558..75e2a517030936 100644 --- a/deps/v8/src/heap/factory-base.cc +++ b/deps/v8/src/heap/factory-base.cc @@ -301,9 +301,9 @@ Handle FactoryBase::NewProtectedWeakFixedArray( } template -Handle FactoryBase::NewByteArray(int length, - AllocationType allocation) { - return ByteArray::New(isolate(), length, allocation); +Handle FactoryBase::NewByteArray( + int length, AllocationType allocation, AllocationAlignment alignment) { + return ByteArray::New(isolate(), length, allocation, alignment); } template @@ -1175,7 +1175,8 @@ inline Handle FactoryBase::SmiToString(Tagged number, if (raw->raw_hash_field() == String::kEmptyHashField && number.value() >= 0) { uint32_t raw_hash_field = StringHasher::MakeArrayIndexHash( - static_cast(number.value()), raw->length()); + static_cast(number.value()), raw->length(), + HashSeed(read_only_roots())); raw->set_raw_hash_field(raw_hash_field); } } @@ -1335,9 +1336,9 @@ FactoryBase::AllocateRawTwoByteInternalizedString( template Tagged FactoryBase::AllocateRawArray( - int size, AllocationType allocation, AllocationHint hint) { - Tagged result = - AllocateRaw(size, allocation, AllocationAlignment::kTaggedAligned, hint); + int size, AllocationType allocation, AllocationHint hint, + AllocationAlignment alignment) { + Tagged result = AllocateRaw(size, allocation, alignment, hint); if ((size > isolate()->heap()->AsHeap()->MaxRegularHeapObjectSize(allocation)) && v8_flags.use_marking_progress_bar) { diff --git a/deps/v8/src/heap/factory-base.h b/deps/v8/src/heap/factory-base.h index 35fa75b80c7949..09cdc50ecd35b4 100644 --- a/deps/v8/src/heap/factory-base.h +++ b/deps/v8/src/heap/factory-base.h @@ -199,7 +199,8 @@ class FactoryBase : public TorqueGeneratedFactory { // The function returns a pre-allocated empty byte array for length = 0. Handle NewByteArray( - int length, AllocationType allocation = AllocationType::kYoung); + int length, AllocationType allocation = AllocationType::kYoung, + AllocationAlignment alignment = kTaggedAligned); // Allocates a trusted byte array in trusted space, initialized with zeros. Handle NewTrustedByteArray( @@ -413,8 +414,10 @@ class FactoryBase : public TorqueGeneratedFactory { static constexpr int kNumberToStringBufferSize = 32; // Allocate memory for an uninitialized array (e.g., a FixedArray or similar). - Tagged AllocateRawArray(int size, AllocationType allocation, - AllocationHint hint = AllocationHint()); + Tagged AllocateRawArray( + int size, AllocationType allocation, + AllocationHint hint = AllocationHint(), + AllocationAlignment alignment = kTaggedAligned); Tagged AllocateRawFixedArray(int length, AllocationType allocation); Tagged AllocateRawWeakArrayList(int length, diff --git a/deps/v8/src/heap/factory.cc b/deps/v8/src/heap/factory.cc index 9f4975f854ddc7..ff7c53d252c8b6 100644 --- a/deps/v8/src/heap/factory.cc +++ b/deps/v8/src/heap/factory.cc @@ -4054,7 +4054,8 @@ Handle Factory::SizeToString(size_t value, bool check_cache) { if (value <= JSArray::kMaxArrayIndex && raw->raw_hash_field() == String::kEmptyHashField) { uint32_t raw_hash_field = StringHasher::MakeArrayIndexHash( - static_cast(value), raw->length()); + static_cast(value), raw->length(), + HashSeed(read_only_roots())); raw->set_raw_hash_field(raw_hash_field); } } diff --git a/deps/v8/src/heap/heap.cc b/deps/v8/src/heap/heap.cc index 1d72853d040bf2..e6b4625684321a 100644 --- a/deps/v8/src/heap/heap.cc +++ b/deps/v8/src/heap/heap.cc @@ -131,7 +131,6 @@ #include "src/tracing/trace-event.h" #include "src/utils/utils-inl.h" #include "src/utils/utils.h" -#include "third_party/rapidhash-v8/secret.h" #if V8_ENABLE_WEBASSEMBLY #include "src/wasm/wasm-engine.h" @@ -6264,31 +6263,6 @@ void Heap::SetUpSpaces() { } } -void Heap::InitializeHashSeed() { - DCHECK(!deserialization_complete_); - uint64_t new_hash_seed; - if (v8_flags.hash_seed == 0) { - int64_t rnd = isolate()->random_number_generator()->NextInt64(); - new_hash_seed = static_cast(rnd); - } else { - new_hash_seed = static_cast(v8_flags.hash_seed); - } - - Tagged hash_seed = ReadOnlyRoots(this).hash_seed(); - - MemCopy(hash_seed->begin(), reinterpret_cast(&new_hash_seed), - kInt64Size); - -#if V8_USE_DEFAULT_HASHER_SECRET - MemCopy(hash_seed->begin() + kInt64Size, - reinterpret_cast(RAPIDHASH_DEFAULT_SECRET), - kInt64Size * 3); -#else - rapidhash_make_secret(new_hash_seed, reinterpret_cast( - hash_seed->begin() + kInt64Size)); -#endif // V8_USE_DEFAULT_HASHER_SECRET -} - std::shared_ptr Heap::GetForegroundTaskRunner( TaskPriority priority) const { return V8::GetCurrentPlatform()->GetForegroundTaskRunner( diff --git a/deps/v8/src/heap/heap.h b/deps/v8/src/heap/heap.h index 0300da2f58e2ee..30bb1a94d0ed5d 100644 --- a/deps/v8/src/heap/heap.h +++ b/deps/v8/src/heap/heap.h @@ -732,9 +732,6 @@ class Heap final { // Prepares the heap, setting up for deserialization. void InitializeMainThreadLocalHeap(LocalHeap* main_thread_local_heap); - // (Re-)Initialize hash seed from flag or RNG. - void InitializeHashSeed(); - // Invoked once for the process from V8::Initialize. static void InitializeOncePerProcess(); diff --git a/deps/v8/src/heap/setup-heap-internal.cc b/deps/v8/src/heap/setup-heap-internal.cc index 00cfa1fd9095c1..fea746467a5718 100644 --- a/deps/v8/src/heap/setup-heap-internal.cc +++ b/deps/v8/src/heap/setup-heap-internal.cc @@ -16,6 +16,7 @@ #include "src/init/heap-symbols.h" #include "src/init/setup-isolate.h" #include "src/interpreter/interpreter.h" +#include "src/numbers/hash-seed.h" #include "src/objects/arguments.h" #include "src/objects/call-site-info.h" #include "src/objects/cell-inl.h" @@ -931,9 +932,10 @@ bool Heap::CreateImportantReadOnlyObjects() { // Hash seed for strings Factory* factory = isolate()->factory(); - set_hash_seed( - *factory->NewByteArray(kInt64Size * 4, AllocationType::kReadOnly)); - InitializeHashSeed(); + set_hash_seed(*factory->NewByteArray(HashSeed::kTotalSize, + AllocationType::kReadOnly, + AllocationAlignment::kDoubleAligned)); + HashSeed::InitializeRoots(isolate()); // Important strings and symbols for (const ConstantStringInit& entry : kImportantConstantStringTable) { diff --git a/deps/v8/src/numbers/hash-seed-inl.h b/deps/v8/src/numbers/hash-seed-inl.h index fb5398692264ef..8988cf8418f7ca 100644 --- a/deps/v8/src/numbers/hash-seed-inl.h +++ b/deps/v8/src/numbers/hash-seed-inl.h @@ -8,7 +8,6 @@ #include "src/numbers/hash-seed.h" #include "src/objects/fixed-array-inl.h" #include "src/roots/roots-inl.h" -#include "third_party/rapidhash-v8/secret.h" namespace v8 { namespace internal { @@ -19,23 +18,22 @@ inline HashSeed::HashSeed(Isolate* isolate) inline HashSeed::HashSeed(LocalIsolate* isolate) : HashSeed(ReadOnlyRoots(isolate)) {} -inline HashSeed::HashSeed(ReadOnlyRoots roots) { - // roots.hash_seed is not aligned - MemCopy(&seed_, roots.hash_seed()->begin(), sizeof(seed_)); - MemCopy(secret_, roots.hash_seed()->begin() + sizeof(seed_), sizeof(secret_)); -} - -inline HashSeed::HashSeed(uint64_t seed, const uint64_t secret[3]) - : seed_(seed), - secret_{ - secret[0], - secret[1], - secret[2], - } {} - -inline HashSeed HashSeed::Default() { - return HashSeed(0, RAPIDHASH_DEFAULT_SECRET); -} +inline HashSeed::HashSeed(ReadOnlyRoots roots) + : data_(reinterpret_cast(roots.hash_seed()->begin())) {} + +inline HashSeed HashSeed::Default() { return HashSeed(kDefaultData); } + +inline uint64_t HashSeed::seed() const { return data_->seed; } +inline const uint64_t* HashSeed::secret() const { return data_->secrets; } + +#ifdef V8_ENABLE_SEEDED_ARRAY_INDEX_HASH +inline uint32_t HashSeed::m1() const { return data_->m1; } +inline uint32_t HashSeed::m1_inv() const { return data_->m1_inv; } +inline uint32_t HashSeed::m2() const { return data_->m2; } +inline uint32_t HashSeed::m2_inv() const { return data_->m2_inv; } +inline uint32_t HashSeed::m3() const { return data_->m3; } +inline uint32_t HashSeed::m3_inv() const { return data_->m3_inv; } +#endif // V8_ENABLE_SEEDED_ARRAY_INDEX_HASH } // namespace internal } // namespace v8 diff --git a/deps/v8/src/numbers/hash-seed.cc b/deps/v8/src/numbers/hash-seed.cc new file mode 100644 index 00000000000000..4251cc3ef15331 --- /dev/null +++ b/deps/v8/src/numbers/hash-seed.cc @@ -0,0 +1,114 @@ +// Copyright 2026 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "src/numbers/hash-seed.h" + +#include + +#include "src/execution/isolate.h" +#include "src/flags/flags.h" +#include "src/numbers/hash-seed-inl.h" +#include "src/objects/name.h" +#include "third_party/rapidhash-v8/secret.h" + +namespace v8 { +namespace internal { + +namespace { + +#ifdef V8_ENABLE_SEEDED_ARRAY_INDEX_HASH +// Calculate the modular inverse using Newton's method. +constexpr uint32_t modular_inverse(uint32_t m) { + uint32_t x = (3 * m) ^ 2; // 5 correct bits + x = x * (2 - m * x); // 10 correct bits + x = x * (2 - m * x); // 20 correct bits + x = x * (2 - m * x); // 40 correct bits + return x; +} + +constexpr uint32_t truncate_for_derived_secrets(uint64_t s) { + return static_cast(s) & Name::kArrayIndexValueMask; +} + +// Derive a multiplier from a rapidhash secret and ensure it's odd. +constexpr uint32_t derive_multiplier(uint64_t secret) { + return truncate_for_derived_secrets(secret) | 1; +} + +// Compute the modular inverse of the derived multiplier. +constexpr uint32_t derive_multiplier_inverse(uint64_t secret) { + return truncate_for_derived_secrets( + modular_inverse(derive_multiplier(secret))); +} + +constexpr bool is_modular_inverse(uint32_t m, uint32_t m_inv) { + return ((m * m_inv) & Name::kArrayIndexValueMask) == 1; +} + +constexpr void DeriveSecretsForArrayIndexHash(HashSeed::Data* data) { + data->m1 = derive_multiplier(data->secrets[0]); + data->m1_inv = derive_multiplier_inverse(data->secrets[0]); + data->m2 = derive_multiplier(data->secrets[1]); + data->m2_inv = derive_multiplier_inverse(data->secrets[1]); + data->m3 = derive_multiplier(data->secrets[2]); + data->m3_inv = derive_multiplier_inverse(data->secrets[2]); +} +#endif // V8_ENABLE_SEEDED_ARRAY_INDEX_HASH + +static constexpr HashSeed::Data kDefaultSeed = [] { + HashSeed::Data d{}; + d.seed = 0; + d.secrets[0] = RAPIDHASH_DEFAULT_SECRET[0]; + d.secrets[1] = RAPIDHASH_DEFAULT_SECRET[1]; + d.secrets[2] = RAPIDHASH_DEFAULT_SECRET[2]; +#ifdef V8_ENABLE_SEEDED_ARRAY_INDEX_HASH + DeriveSecretsForArrayIndexHash(&d); +#endif // V8_ENABLE_SEEDED_ARRAY_INDEX_HASH + return d; +}(); + +} // anonymous namespace + +static_assert(HashSeed::kSecretsCount == arraysize(RAPIDHASH_DEFAULT_SECRET)); +const HashSeed::Data* const HashSeed::kDefaultData = &kDefaultSeed; + +#ifdef V8_ENABLE_SEEDED_ARRAY_INDEX_HASH +// Compile-time verification that m * m_inv === 1 for the derived secrets. +static_assert(is_modular_inverse(kDefaultSeed.m1, kDefaultSeed.m1_inv)); +static_assert(is_modular_inverse(kDefaultSeed.m2, kDefaultSeed.m2_inv)); +static_assert(is_modular_inverse(kDefaultSeed.m3, kDefaultSeed.m3_inv)); +#endif // V8_ENABLE_SEEDED_ARRAY_INDEX_HASH + +// static +void HashSeed::InitializeRoots(Isolate* isolate) { + DCHECK(!isolate->heap()->deserialization_complete()); + uint64_t seed; + if (v8_flags.hash_seed == 0) { + int64_t rnd = isolate->random_number_generator()->NextInt64(); + seed = static_cast(rnd); + } else { + seed = static_cast(v8_flags.hash_seed); + } + + // Write the seed and derived secrets into the read-only roots ByteArray. + Data* data = const_cast(HashSeed(isolate).data_); + +#if V8_USE_DEFAULT_HASHER_SECRET + // Copy from the default seed and just override the meta seed. + *data = kDefaultSeed; + data->seed = seed; +#else + data->seed = seed; + rapidhash_make_secret(seed, data->secrets); +#ifdef V8_ENABLE_SEEDED_ARRAY_INDEX_HASH + DeriveSecretsForArrayIndexHash(data); + DCHECK(is_modular_inverse(data->m1, data->m1_inv)); + DCHECK(is_modular_inverse(data->m2, data->m2_inv)); + DCHECK(is_modular_inverse(data->m3, data->m3_inv)); +#endif // V8_ENABLE_SEEDED_ARRAY_INDEX_HASH +#endif // V8_USE_DEFAULT_HASHER_SECRET +} + +} // namespace internal +} // namespace v8 diff --git a/deps/v8/src/numbers/hash-seed.h b/deps/v8/src/numbers/hash-seed.h index 48368a2512056f..7cabdf1223ce78 100644 --- a/deps/v8/src/numbers/hash-seed.h +++ b/deps/v8/src/numbers/hash-seed.h @@ -5,7 +5,11 @@ #ifndef V8_NUMBERS_HASH_SEED_H_ #define V8_NUMBERS_HASH_SEED_H_ -#include +#include +#include +#include + +#include "src/base/macros.h" namespace v8 { namespace internal { @@ -14,28 +18,84 @@ class Isolate; class LocalIsolate; class ReadOnlyRoots; -class HashSeed { +// A lightweight view over the hash_seed ByteArray in read-only roots. +class V8_EXPORT_PRIVATE HashSeed { public: inline explicit HashSeed(Isolate* isolate); inline explicit HashSeed(LocalIsolate* isolate); inline explicit HashSeed(ReadOnlyRoots roots); - inline explicit HashSeed(uint64_t seed, const uint64_t secret[3]); static inline HashSeed Default(); - uint64_t seed() const { return seed_; } - const uint64_t* secret() const { return secret_; } + inline uint64_t seed() const; + inline const uint64_t* secret() const; + + bool operator==(const HashSeed& b) const { return data_ == b.data_; } + + static constexpr int kSecretsCount = 3; + + // The ReadOnlyRoots::hash_seed() byte array can be interpreted + // as a HashSeed::Data struct. + // Since this maps over either the read-only roots or over a static byte + // array, and in both cases, must be allocated at 8-byte boundaries, + // we don't use V8_OBJECT here. + struct Data { + // meta seed from --hash-seed, 0 = generate at startup + uint64_t seed; + // When V8_USE_DEFAULT_HASHER_SECRET is enabled, these are just + // RAPIDHASH_DEFAULT_SECRET. Otherwise they are derived from the seed + // using rapidhash_make_secret(). + uint64_t secrets[kSecretsCount]; + +#ifdef V8_ENABLE_SEEDED_ARRAY_INDEX_HASH + // Additional precomputed secrets for seeding the array index value hashes. + uint32_t m1; // lower kArrayIndexValueBits bits of secret[0], must be odd + uint32_t m1_inv; // modular inverse of m1 mod 2^kArrayIndexValueBits + uint32_t m2; // lower kArrayIndexValueBits bits of secret[1], must be odd + uint32_t m2_inv; // modular inverse of m2 mod 2^kArrayIndexValueBits + uint32_t m3; // lower kArrayIndexValueBits bits of secret[2], must be odd + uint32_t m3_inv; // modular inverse of m3 mod 2^kArrayIndexValueBits +#endif // V8_ENABLE_SEEDED_ARRAY_INDEX_HASH + }; - constexpr bool operator==(const HashSeed& b) const { - return seed_ == b.seed_ && secret_[0] == b.secret_[0] && - secret_[1] == b.secret_[1] && secret_[2] == b.secret_[2]; - } + static constexpr int kTotalSize = sizeof(Data); + +#ifdef V8_ENABLE_SEEDED_ARRAY_INDEX_HASH + // Byte offsets from the data start, for CSA that loads fields at raw + // offsets from the ByteArray data start. + static constexpr int kDerivedM1Offset = offsetof(Data, m1); + static constexpr int kDerivedM1InvOffset = offsetof(Data, m1_inv); + static constexpr int kDerivedM2Offset = offsetof(Data, m2); + static constexpr int kDerivedM2InvOffset = offsetof(Data, m2_inv); + static constexpr int kDerivedM3Offset = offsetof(Data, m3); + static constexpr int kDerivedM3InvOffset = offsetof(Data, m3_inv); + + inline uint32_t m1() const; + inline uint32_t m1_inv() const; + inline uint32_t m2() const; + inline uint32_t m2_inv() const; + inline uint32_t m3() const; + inline uint32_t m3_inv() const; +#endif // V8_ENABLE_SEEDED_ARRAY_INDEX_HASH + + // Generates a hash seed (from --hash-seed or the RNG) and writes it + // together with derived secrets into the isolate's hash_seed in + // its read-only roots. + static void InitializeRoots(Isolate* isolate); private: - uint64_t seed_; - uint64_t secret_[3]; + // Pointer into the Data overlaying the ByteArray data (either + // points to read-only roots or to kDefaultData). + const Data* data_; + explicit HashSeed(const Data* data) : data_(data) {} + + // Points to the static constexpr default seed. + static const Data* const kDefaultData; }; +static_assert(std::is_trivially_copyable_v); +static_assert(alignof(HashSeed::Data) == alignof(uint64_t)); + } // namespace internal } // namespace v8 diff --git a/deps/v8/src/objects/fixed-array-inl.h b/deps/v8/src/objects/fixed-array-inl.h index 59fdf98dbb1521..7c581430c0ba9c 100644 --- a/deps/v8/src/objects/fixed-array-inl.h +++ b/deps/v8/src/objects/fixed-array-inl.h @@ -579,15 +579,15 @@ template Handle PrimitiveArrayBase::Allocate( IsolateT* isolate, int length, std::optional* no_gc_out, - AllocationType allocation) { + AllocationType allocation, AllocationAlignment alignment) { // Note 0-length is explicitly allowed since not all subtypes can be // assumed to have canonical 0-length instances. DCHECK_GE(length, 0); DCHECK_LE(length, kMaxLength); DCHECK(!no_gc_out->has_value()); - Tagged xs = UncheckedCast( - isolate->factory()->AllocateRawArray(SizeFor(length), allocation)); + Tagged xs = UncheckedCast(isolate->factory()->AllocateRawArray( + SizeFor(length), allocation, AllocationHint(), alignment)); ReadOnlyRoots roots{isolate}; if (DEBUG_BOOL) no_gc_out->emplace(); @@ -805,7 +805,8 @@ DirectHandle ArrayList::New(IsolateT* isolate, int capacity, // static template Handle ByteArray::New(IsolateT* isolate, int length, - AllocationType allocation) { + AllocationType allocation, + AllocationAlignment alignment) { if (V8_UNLIKELY(static_cast(length) > kMaxLength)) { base::FatalNoSecurityImpact("Fatal JavaScript invalid size error %d", length); @@ -815,7 +816,7 @@ Handle ByteArray::New(IsolateT* isolate, int length, std::optional no_gc; Handle result = - Cast(Allocate(isolate, length, &no_gc, allocation)); + Cast(Allocate(isolate, length, &no_gc, allocation, alignment)); int padding_size = SizeFor(length) - OffsetOfElementAt(length); memset(&result->values()[length], 0, padding_size); diff --git a/deps/v8/src/objects/fixed-array.h b/deps/v8/src/objects/fixed-array.h index ace95d4c7ee0a6..c14f322c303f61 100644 --- a/deps/v8/src/objects/fixed-array.h +++ b/deps/v8/src/objects/fixed-array.h @@ -445,7 +445,8 @@ class PrimitiveArrayBase : public detail::ArrayHeaderBase { static Handle Allocate( IsolateT* isolate, int length, std::optional* no_gc_out, - AllocationType allocation = AllocationType::kYoung); + AllocationType allocation = AllocationType::kYoung, + AllocationAlignment alignment = kTaggedAligned); inline bool IsInBounds(int index) const; @@ -782,7 +783,8 @@ V8_OBJECT class ByteArray template static inline Handle New( IsolateT* isolate, int capacity, - AllocationType allocation = AllocationType::kYoung); + AllocationType allocation = AllocationType::kYoung, + AllocationAlignment alignment = kTaggedAligned); inline uint32_t get_int(int offset) const; inline void set_int(int offset, uint32_t value); diff --git a/deps/v8/src/objects/name.h b/deps/v8/src/objects/name.h index d5de0f013b91c9..314ea79ec2a066 100644 --- a/deps/v8/src/objects/name.h +++ b/deps/v8/src/objects/name.h @@ -167,7 +167,19 @@ V8_OBJECT class Name : public PrimitiveHeapObject { // For strings which are array indexes the hash value has the string length // mixed into the hash, mainly to avoid a hash value of zero which would be // the case for the string '0'. 24 bits are used for the array index value. - static const int kArrayIndexValueBits = 24; + static constexpr int kArrayIndexValueBits = 24; + // Mask for extracting the lower kArrayIndexValueBits of a value. + static constexpr uint32_t kArrayIndexValueMask = + (1u << kArrayIndexValueBits) - 1; +#ifdef V8_ENABLE_SEEDED_ARRAY_INDEX_HASH + // Half-width shift used by the seeded xorshift-multiply mixing. + static constexpr int kArrayIndexHashShift = kArrayIndexValueBits / 2; + // The shift must be at least the half width for the xorshift to be an + // involution. + static_assert(kArrayIndexHashShift * 2 >= kArrayIndexValueBits, + "kArrayIndexHashShift must be at least half of " + "kArrayIndexValueBits"); +#endif // V8_ENABLE_SEEDED_ARRAY_INDEX_HASH static const int kArrayIndexLengthBits = kBitsPerInt - kArrayIndexValueBits - HashFieldTypeBits::kSize; diff --git a/deps/v8/src/objects/name.tq b/deps/v8/src/objects/name.tq index 5c4259afbc487d..331a1557f69f2e 100644 --- a/deps/v8/src/objects/name.tq +++ b/deps/v8/src/objects/name.tq @@ -56,6 +56,11 @@ macro ContainsCachedArrayIndex(hash: uint32): bool { return (hash & kDoesNotContainCachedArrayIndexMask) == 0; } +@if(V8_ENABLE_SEEDED_ARRAY_INDEX_HASH) + extern macro SeedArrayIndexValue(uint32): uint32; +@if(V8_ENABLE_SEEDED_ARRAY_INDEX_HASH) + extern macro UnseedArrayIndexValue(uint32): uint32; + const kArrayIndexValueBitsShift: uint32 = kNofHashBitFields; const kArrayIndexLengthBitsShift: uint32 = kNofHashBitFields + kArrayIndexValueBits; @@ -73,8 +78,10 @@ macro IsIntegerIndex(hash: NameHash): bool { return hash.hash_field_type == HashFieldType::kIntegerIndex; } -macro MakeArrayIndexHash(value: uint32, length: uint32): NameHash { - // This is in sync with StringHasher::MakeArrayIndexHash. +// This is in sync with the private StringHasher::MakeArrayIndexHash without +// seeding. Do not call directly, use the @export MakeArrayIndexHash wrapper +// below. +macro MakeArrayIndexHashRaw(value: uint32, length: uint32): NameHash { dcheck(length <= kMaxArrayIndexSize); const one: uint32 = 1; dcheck(TenToThe(kMaxCachedArrayIndexLength) < (one << kArrayIndexValueBits)); @@ -88,3 +95,35 @@ macro MakeArrayIndexHash(value: uint32, length: uint32): NameHash { dcheck(IsIntegerIndex(hash)); return hash; } + +// This is in sync with the private StringHasher::DecodeArrayIndexFromHashField +// without seeding. Do not call directly, use the @export +// DecodeArrayIndexFromHashField wrapper below. +macro DecodeArrayIndexFromHashFieldRaw(rawHashField: uint32): uint32 { + const hash: NameHash = %RawDownCast(rawHashField); + dcheck(ContainsCachedArrayIndex(rawHashField) || IsIntegerIndex(hash)); + return hash.array_index_value; +} + +// Mirror C++ public StringHasher::MakeArrayIndexHash. +@export +macro MakeArrayIndexHash(value: uint32, length: uint32): NameHash { + @if(V8_ENABLE_SEEDED_ARRAY_INDEX_HASH) { + return MakeArrayIndexHashRaw(SeedArrayIndexValue(value), length); + } + @ifnot(V8_ENABLE_SEEDED_ARRAY_INDEX_HASH) { + return MakeArrayIndexHashRaw(value, length); + } +} + +// Mirror C++ public StringHasher::DecodeArrayIndexFromHashField. +@export +macro DecodeArrayIndexFromHashField(rawHashField: uint32): uint32 { + const value: uint32 = DecodeArrayIndexFromHashFieldRaw(rawHashField); + @if(V8_ENABLE_SEEDED_ARRAY_INDEX_HASH) { + return UnseedArrayIndexValue(value); + } + @ifnot(V8_ENABLE_SEEDED_ARRAY_INDEX_HASH) { + return value; + } +} diff --git a/deps/v8/src/objects/string-inl.h b/deps/v8/src/objects/string-inl.h index e80cec42c33bd2..6d72fdacd68268 100644 --- a/deps/v8/src/objects/string-inl.h +++ b/deps/v8/src/objects/string-inl.h @@ -1766,7 +1766,8 @@ bool String::AsArrayIndex(uint32_t* index) { DisallowGarbageCollection no_gc; uint32_t field = raw_hash_field(); if (ContainsCachedArrayIndex(field)) { - *index = ArrayIndexValueBits::decode(field); + *index = StringHasher::DecodeArrayIndexFromHashField( + field, HashSeed(EarlyGetReadOnlyRoots())); return true; } if (IsHashFieldComputed(field) && !IsIntegerIndex(field)) { @@ -1778,7 +1779,8 @@ bool String::AsArrayIndex(uint32_t* index) { bool String::AsIntegerIndex(size_t* index) { uint32_t field = raw_hash_field(); if (ContainsCachedArrayIndex(field)) { - *index = ArrayIndexValueBits::decode(field); + *index = StringHasher::DecodeArrayIndexFromHashField( + field, HashSeed(EarlyGetReadOnlyRoots())); return true; } if (IsHashFieldComputed(field) && !IsIntegerIndex(field)) { diff --git a/deps/v8/src/objects/string-table.cc b/deps/v8/src/objects/string-table.cc index 98ed901bdb630c..8df76934862b1d 100644 --- a/deps/v8/src/objects/string-table.cc +++ b/deps/v8/src/objects/string-table.cc @@ -619,7 +619,8 @@ Address StringTable::Data::TryStringToIndexOrLookupExisting( // String could be an array index. if (Name::ContainsCachedArrayIndex(raw_hash_field)) { - return Smi::FromInt(String::ArrayIndexValueBits::decode(raw_hash_field)) + return Smi::FromInt(StringHasher::DecodeArrayIndexFromHashField( + raw_hash_field, seed)) .ptr(); } diff --git a/deps/v8/src/objects/string.cc b/deps/v8/src/objects/string.cc index 7ca167e81d22c3..9b6287a4d5cf30 100644 --- a/deps/v8/src/objects/string.cc +++ b/deps/v8/src/objects/string.cc @@ -1905,7 +1905,8 @@ bool String::SlowAsArrayIndex(uint32_t* index) { if (length <= kMaxCachedArrayIndexLength) { uint32_t field = EnsureRawHash(); // Force computation of hash code. if (!IsIntegerIndex(field)) return false; - *index = ArrayIndexValueBits::decode(field); + *index = StringHasher::DecodeArrayIndexFromHashField( + field, HashSeed(EarlyGetReadOnlyRoots())); return true; } if (length == 0 || length > kMaxArrayIndexSize) return false; @@ -1919,7 +1920,8 @@ bool String::SlowAsIntegerIndex(size_t* index) { if (length <= kMaxCachedArrayIndexLength) { uint32_t field = EnsureRawHash(); // Force computation of hash code. if (!IsIntegerIndex(field)) return false; - *index = ArrayIndexValueBits::decode(field); + *index = StringHasher::DecodeArrayIndexFromHashField( + field, HashSeed(EarlyGetReadOnlyRoots())); return true; } if (length == 0 || length > kMaxIntegerIndexSize) return false; diff --git a/deps/v8/src/snapshot/read-only-deserializer.cc b/deps/v8/src/snapshot/read-only-deserializer.cc index 719b7fbda0885a..ab8f2a1eeec8e4 100644 --- a/deps/v8/src/snapshot/read-only-deserializer.cc +++ b/deps/v8/src/snapshot/read-only-deserializer.cc @@ -8,6 +8,7 @@ #include "src/heap/heap-inl.h" #include "src/heap/read-only-heap.h" #include "src/logging/counters-scopes.h" +#include "src/numbers/hash-seed.h" #include "src/objects/objects-inl.h" #include "src/objects/slots.h" #include "src/snapshot/embedded/embedded-data-inl.h" @@ -174,7 +175,7 @@ void ReadOnlyDeserializer::DeserializeIntoIsolate() { #endif if (should_rehash()) { - isolate()->heap()->InitializeHashSeed(); + HashSeed::InitializeRoots(isolate()); Rehash(); } diff --git a/deps/v8/src/snapshot/shared-heap-deserializer.cc b/deps/v8/src/snapshot/shared-heap-deserializer.cc index bfb4a8948d5339..1c881843e4c54a 100644 --- a/deps/v8/src/snapshot/shared-heap-deserializer.cc +++ b/deps/v8/src/snapshot/shared-heap-deserializer.cc @@ -29,7 +29,7 @@ void SharedHeapDeserializer::DeserializeIntoIsolate() { if (should_rehash()) { // The hash seed has already been initialized in ReadOnlyDeserializer, thus - // there is no need to call `isolate()->heap()->InitializeHashSeed();`. + // there is no need to call `HashSeed::InitializeRoots(isolate());`. Rehash(); } } diff --git a/deps/v8/src/strings/string-hasher-inl.h b/deps/v8/src/strings/string-hasher-inl.h index fe4d466ad9e298..39f99d701cb5e8 100644 --- a/deps/v8/src/strings/string-hasher-inl.h +++ b/deps/v8/src/strings/string-hasher-inl.h @@ -8,9 +8,6 @@ #include "src/strings/string-hasher.h" // Include the non-inl header before the rest of the headers. -#include "src/common/globals.h" -#include "src/utils/utils.h" - #ifdef __SSE2__ #include #elif defined(__ARM_NEON__) @@ -20,10 +17,12 @@ // Comment inserted to prevent header reordering. #include +#include "src/common/globals.h" #include "src/objects/name-inl.h" #include "src/objects/string-inl.h" #include "src/strings/char-predicates-inl.h" #include "src/utils/utils-inl.h" +#include "src/utils/utils.h" #include "third_party/rapidhash-v8/rapidhash.h" namespace v8 { @@ -116,6 +115,70 @@ uint32_t StringHasher::MakeArrayIndexHash(uint32_t value, uint32_t length) { return value; } +uint32_t StringHasher::DecodeArrayIndexFromHashField(uint32_t raw_hash_field) { + DCHECK(String::ContainsCachedArrayIndex(raw_hash_field) || + String::IsIntegerIndex(raw_hash_field)); + return String::ArrayIndexValueBits::decode(raw_hash_field); +} + +#ifdef V8_ENABLE_SEEDED_ARRAY_INDEX_HASH +uint32_t StringHasher::SeedArrayIndexValue(uint32_t value, + const HashSeed seed) { + uint32_t m1 = seed.m1(); + uint32_t m2 = seed.m2(); + uint32_t m3 = seed.m3(); + constexpr uint32_t kShift = Name::kArrayIndexHashShift; + constexpr uint32_t kMask = Name::kArrayIndexValueMask; + // 3-round xorshift-multiply. + uint32_t x = value; + x ^= x >> kShift; + x = (x * m1) & kMask; + x ^= x >> kShift; + x = (x * m2) & kMask; + x ^= x >> kShift; + x = (x * m3) & kMask; + x ^= x >> kShift; + return x; +} + +uint32_t StringHasher::UnseedArrayIndexValue(uint32_t value, + const HashSeed seed) { + uint32_t m1_inv = seed.m1_inv(); + uint32_t m2_inv = seed.m2_inv(); + uint32_t m3_inv = seed.m3_inv(); + uint32_t x = value; + constexpr uint32_t kShift = Name::kArrayIndexHashShift; + constexpr uint32_t kMask = Name::kArrayIndexValueMask; + // 3-round xorshift-multiply (inverse). + // Xorshift is an involution when kShift is at least half of the value width. + x ^= x >> kShift; + x = (x * m3_inv) & kMask; + x ^= x >> kShift; + x = (x * m2_inv) & kMask; + x ^= x >> kShift; + x = (x * m1_inv) & kMask; + x ^= x >> kShift; + return x; +} +#endif // V8_ENABLE_SEEDED_ARRAY_INDEX_HASH + +uint32_t StringHasher::MakeArrayIndexHash( + uint32_t value, uint32_t length, [[maybe_unused]] const HashSeed seed) { +#ifdef V8_ENABLE_SEEDED_ARRAY_INDEX_HASH + value = SeedArrayIndexValue(value, seed); +#endif + return MakeArrayIndexHash(value, length); +} + +uint32_t StringHasher::DecodeArrayIndexFromHashField( + uint32_t raw_hash_field, [[maybe_unused]] const HashSeed seed) { + uint32_t value = DecodeArrayIndexFromHashField(raw_hash_field); +#ifdef V8_ENABLE_SEEDED_ARRAY_INDEX_HASH + value = UnseedArrayIndexValue(value, seed); +#endif + return value; +} + namespace detail { enum IndexParseResult { kSuccess, kNonIndex, kOverflow }; @@ -239,9 +302,11 @@ uint32_t StringHasher::HashSequentialString(const char_t* chars_raw, detail::ArrayIndexT index; uint32_t i; switch (detail::TryParseArrayIndex(chars, length, i, index)) { - case detail::kSuccess: + case detail::kSuccess: { DCHECK_LE(index, String::kMaxArrayIndex); - return MakeArrayIndexHash(static_cast(index), length); + return StringHasher::MakeArrayIndexHash(static_cast(index), + length, seed); + } case detail::kNonIndex: // A non-index result from TryParseArrayIndex means we don't need to // check for integer indices. diff --git a/deps/v8/src/strings/string-hasher.h b/deps/v8/src/strings/string-hasher.h index fe824d9bed4b3c..02baa5825c39b2 100644 --- a/deps/v8/src/strings/string-hasher.h +++ b/deps/v8/src/strings/string-hasher.h @@ -40,10 +40,23 @@ class V8_EXPORT_PRIVATE StringHasher final { uint32_t length, const HashSeed seed); - // Calculated hash value for a string consisting of 1 to + // Calculate the hash value for a string consisting of 1 to // String::kMaxArrayIndexSize digits with no leading zeros (except "0"). - // value is represented decimal value. - static V8_INLINE uint32_t MakeArrayIndexHash(uint32_t value, uint32_t length); + // + // The entire hash field consists of (from least significant bit to most): + // - HashFieldType::kIntegerIndex + // - kArrayIndexValueBits::kSize bits containing the hash value + // - The length of the decimal string + // + // When V8_ENABLE_SEEDED_ARRAY_INDEX_HASH is enabled, the numeric value + // is scrambled using secrets derived from the hash seed. When it's disabled + // the public overloads ignore the seed, whose retrieval should be optimized + // away in common configurations. + static V8_INLINE uint32_t MakeArrayIndexHash(uint32_t value, uint32_t length, + const HashSeed seed); + // Decode array index value from raw hash field and reverse seeding, if any. + static V8_INLINE uint32_t + DecodeArrayIndexFromHashField(uint32_t raw_hash_field, const HashSeed seed); // No string is allowed to have a hash of zero. That value is reserved // for internal properties. If the hash calculation yields zero then we @@ -51,6 +64,40 @@ class V8_EXPORT_PRIVATE StringHasher final { static const int kZeroHash = 27; static V8_INLINE uint32_t GetTrivialHash(uint32_t length); + + private: + // Raw encode/decode without seeding. Use the public overloads above. + static V8_INLINE uint32_t MakeArrayIndexHash(uint32_t value, uint32_t length); + static V8_INLINE uint32_t + DecodeArrayIndexFromHashField(uint32_t raw_hash_field); + +#ifdef V8_ENABLE_SEEDED_ARRAY_INDEX_HASH + // When V8_ENABLE_SEEDED_ARRAY_INDEX_HASH is enabled, the numeric value + // will be scrambled with 3 rounds of xorshift-multiply. + // + // x ^= x >> kShift; x = (x * m1) & kMask; // round 1 + // x ^= x >> kShift; x = (x * m2) & kMask; // round 2 + // x ^= x >> kShift; x = (x * m3) & kMask; // round 3 + // x ^= x >> kShift; // finalize + // + // To decode, apply the same steps with the modular inverses of m1, m2 + // and m3 in reverse order. + // + // x ^= x >> kShift; x = (x * m3_inv) & kMask; // round 1 + // x ^= x >> kShift; x = (x * m2_inv) & kMask; // round 2 + // x ^= x >> kShift; x = (x * m1_inv) & kMask; // round 3 + // x ^= x >> kShift; // finalize + // + // where kShift = kArrayIndexValueBits / 2, kMask = kArrayIndexValueMask, + // m1, m2, m3 (all odd) are derived from the Isolate's rapidhash secrets. + // m1_inv, m2_inv, m3_inv (modular inverses) are precomputed so that + // UnseedArrayIndexValue can quickly recover the original value. + static V8_INLINE uint32_t SeedArrayIndexValue(uint32_t value, + const HashSeed seed); + // Decode array index value from seeded raw hash field. + static V8_INLINE uint32_t UnseedArrayIndexValue(uint32_t value, + const HashSeed seed); +#endif // V8_ENABLE_SEEDED_ARRAY_INDEX_HASH }; // Useful for std containers that require something ()'able. diff --git a/deps/v8/src/torque/torque-parser.cc b/deps/v8/src/torque/torque-parser.cc index f55d915c22ef9a..bebbe2c05d9240 100644 --- a/deps/v8/src/torque/torque-parser.cc +++ b/deps/v8/src/torque/torque-parser.cc @@ -86,6 +86,11 @@ class BuildFlags : public base::ContextualClass { build_flags_["V8_ENABLE_DRUMBRAKE"] = true; #else build_flags_["V8_ENABLE_DRUMBRAKE"] = false; +#endif +#ifdef V8_ENABLE_SEEDED_ARRAY_INDEX_HASH + build_flags_["V8_ENABLE_SEEDED_ARRAY_INDEX_HASH"] = true; +#else + build_flags_["V8_ENABLE_SEEDED_ARRAY_INDEX_HASH"] = false; #endif } static bool GetFlag(const std::string& name, const char* production) { diff --git a/deps/v8/test/cctest/test-strings.cc b/deps/v8/test/cctest/test-strings.cc index 6d2100d83f9b18..80695f664022cd 100644 --- a/deps/v8/test/cctest/test-strings.cc +++ b/deps/v8/test/cctest/test-strings.cc @@ -1854,38 +1854,67 @@ TEST(HashArrayIndexStrings) { v8::HandleScope scope(CcTest::isolate()); i::Isolate* isolate = CcTest::i_isolate(); - CHECK_EQ(Name::HashBits::decode( - StringHasher::MakeArrayIndexHash(0 /* value */, 1 /* length */)), + i::HashSeed seed(isolate); + CHECK_EQ(Name::HashBits::decode(StringHasher::MakeArrayIndexHash( + 0 /* value */, 1 /* length */, seed)), isolate->factory()->zero_string()->hash()); - CHECK_EQ(Name::HashBits::decode( - StringHasher::MakeArrayIndexHash(1 /* value */, 1 /* length */)), + CHECK_EQ(Name::HashBits::decode(StringHasher::MakeArrayIndexHash( + 1 /* value */, 1 /* length */, seed)), isolate->factory()->one_string()->hash()); + CHECK_EQ(0u, StringHasher::DecodeArrayIndexFromHashField( + isolate->factory()->zero_string()->raw_hash_field(), seed)); + CHECK_EQ(1u, StringHasher::DecodeArrayIndexFromHashField( + isolate->factory()->one_string()->raw_hash_field(), seed)); + IndexData tests[] = { - {"", false, 0, false, 0}, - {"123no", false, 0, false, 0}, - {"12345", true, 12345, true, 12345}, - {"12345678", true, 12345678, true, 12345678}, - {"4294967294", true, 4294967294u, true, 4294967294u}, + {"", false, 0, false, 0}, + {"123no", false, 0, false, 0}, + {"12345", true, 12345, true, 12345}, + {"12345678", true, 12345678, true, 12345678}, + {"1000000", true, 1000000, true, 1000000}, + {"9999999", true, 9999999, true, 9999999}, + {"10000000", true, 10000000, true, 10000000}, + {"16777215", true, 16777215, true, 16777215}, // max cached index + {"99999999", true, 99999999, true, 99999999}, + {"4294967294", true, 4294967294u, true, 4294967294u}, #if V8_TARGET_ARCH_32_BIT - {"4294967295", false, 0, false, 0}, // Valid length but not index. - {"4294967296", false, 0, false, 0}, - {"9007199254740991", false, 0, false, 0}, + {"4294967295", false, 0, false, 0}, // Valid length but not index. + {"4294967296", false, 0, false, 0}, + {"9007199254740991", false, 0, false, 0}, #else - {"4294967295", false, 0, true, 4294967295u}, - {"4294967296", false, 0, true, 4294967296ull}, - {"9007199254740991", false, 0, true, 9007199254740991ull}, + {"4294967295", false, 0, true, 4294967295u}, + {"4294967296", false, 0, true, 4294967296ull}, + {"9007199254740991", false, 0, true, 9007199254740991ull}, #endif - {"9007199254740992", false, 0, false, 0}, - {"18446744073709551615", false, 0, false, 0}, - {"18446744073709551616", false, 0, false, 0} - }; + {"9007199254740992", false, 0, false, 0}, + {"18446744073709551615", false, 0, false, 0}, + {"18446744073709551616", false, 0, false, 0}}; for (int i = 0, n = arraysize(tests); i < n; i++) { TestString(isolate, tests[i]); } } +TEST(ArrayIndexHashRoundTrip) { + CcTest::InitializeVM(); + LocalContext context; + v8::HandleScope scope(CcTest::isolate()); + i::Isolate* isolate = CcTest::i_isolate(); + i::HashSeed seed(isolate); + + constexpr uint32_t max_value = (1u << Name::kArrayIndexValueBits) - 1; + for (uint32_t value = 0; value <= max_value; value++) { + uint32_t length = + value == 0 ? 1 : static_cast(std::log10(value)) + 1; + uint32_t raw_hash_field = + StringHasher::MakeArrayIndexHash(value, length, seed); + uint32_t decoded = + StringHasher::DecodeArrayIndexFromHashField(raw_hash_field, seed); + CHECK_EQ(value, decoded); + } +} + TEST(StringEquals) { v8::Isolate* isolate = CcTest::isolate(); v8::HandleScope scope(isolate); diff --git a/test/fixtures/array-hash-collision.js b/test/fixtures/array-hash-collision.js new file mode 100644 index 00000000000000..b803de6b9d4b57 --- /dev/null +++ b/test/fixtures/array-hash-collision.js @@ -0,0 +1,27 @@ +'use strict'; + +// See https://hackerone.com/reports/3511792 + +const payload = []; +const val = 1234; +const MOD = 2 ** 19; +const CHN = 2 ** 17; +const REP = 2 ** 17; + +if (process.argv[2] === 'benign') { + for (let i = 0; i < CHN + REP; i++) { + payload.push(`${val + i}`); + } +} else { + let j = val + MOD; + for (let i = 1; i < CHN; i++) { + payload.push(`${j}`); + j = (j + i) % MOD; + } + for (let k = 0; k < REP; k++) { + payload.push(`${val}`); + } +} + +const string = JSON.stringify({ data: payload }); +JSON.parse(string); diff --git a/test/pummel/test-array-hash-collision.js b/test/pummel/test-array-hash-collision.js new file mode 100644 index 00000000000000..8e3337818ac7de --- /dev/null +++ b/test/pummel/test-array-hash-collision.js @@ -0,0 +1,27 @@ +'use strict'; + +// This is a regression test for https://hackerone.com/reports/3511792 + +require('../common'); +const assert = require('assert'); +const { spawnSync } = require('child_process'); +const { performance } = require('perf_hooks'); +const fixtures = require('../common/fixtures'); + +const fixturePath = fixtures.path('array-hash-collision.js'); + +const t0 = performance.now(); +const benignResult = spawnSync(process.execPath, [fixturePath, 'benign']); +const benignTime = performance.now() - t0; +assert.strictEqual(benignResult.status, 0); +console.log(`Benign test completed in ${benignTime.toFixed(2)}ms.`); + +const t1 = performance.now(); +const maliciousResult = spawnSync(process.execPath, [fixturePath, 'malicious'], { + timeout: Math.ceil(benignTime * 10), +}); +const maliciousTime = performance.now() - t1; +console.log(`Malicious test completed in ${maliciousTime.toFixed(2)}ms.`); + +assert.strictEqual(maliciousResult.status, 0, `Hash flooding regression detected: ` + + `Benign took ${benignTime}ms, malicious took more than ${maliciousTime}ms.`); diff --git a/tools/v8_gypfiles/features.gypi b/tools/v8_gypfiles/features.gypi index c2805da739ce2a..ed9a5a5c487157 100644 --- a/tools/v8_gypfiles/features.gypi +++ b/tools/v8_gypfiles/features.gypi @@ -197,6 +197,9 @@ # Use Siphash as added protection against hash flooding attacks. 'v8_use_siphash%': 0, + # Enable seeded array index hash. + 'v8_enable_seeded_array_index_hash%': 1, + # Use Perfetto (https://perfetto.dev) as the default TracingController. Not # currently implemented. 'v8_use_perfetto%': 0, @@ -442,6 +445,9 @@ ['v8_use_siphash==1', { 'defines': ['V8_USE_SIPHASH',], }], + ['v8_enable_seeded_array_index_hash==1', { + 'defines': ['V8_ENABLE_SEEDED_ARRAY_INDEX_HASH',], + }], ['dcheck_always_on!=0', { 'defines': ['DEBUG',], }, {