Skip to content

Commit 3bce2b1

Browse files
ljoy913ljoy913
committed
Harden worker dispatch and fix ReDoS; build/CI cleanup
- Validate dynamic operation/algorithm dispatch in the worker using own-property (hasOwnProperty) and function-type checks so user-controlled names cannot reach inherited Object.prototype members (resolves CodeQL unvalidated dynamic method call). - Replace the stack-trace parsing regex in global.js with a non-backtracking pattern (resolves CodeQL ReDoS). - Remove dead code and add fs.watch recursive fallback in build.mjs. - Bump publish workflow to actions/checkout@v5, setup-node@v5, Node 22.
1 parent 34ed498 commit 3bce2b1

8 files changed

Lines changed: 87 additions & 24 deletions

File tree

‎.github/workflows/publish.yml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,12 @@ jobs:
2929
runs-on: ubuntu-latest
3030
steps:
3131
- name: Checkout
32-
uses: actions/checkout@v4
32+
uses: actions/checkout@v5
3333

3434
- name: Setup Node.js
35-
uses: actions/setup-node@v4
35+
uses: actions/setup-node@v5
3636
with:
37-
node-version: 20
37+
node-version: 22
3838
registry-url: https://registry.npmjs.org
3939

4040
- name: Install dependencies

‎build.mjs‎

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,6 @@ async function build() {
328328
}
329329

330330
async function watch() {
331-
const chokidar = await import("node:fs/promises");
332331
const { watch: fsWatch } = await import("node:fs");
333332
const all = new Set([...fullBuild, LICENSE_FILE]);
334333
let timer = null;
@@ -340,16 +339,35 @@ async function watch() {
340339
};
341340
await build();
342341
console.log("watching for changes...");
342+
343+
// Keep references to every FSWatcher so they are not garbage-collected
344+
// and stay active for the lifetime of the process.
345+
const watchers = [];
346+
343347
for (const f of all) {
344348
try {
345-
fsWatch(f, rebuild);
349+
watchers.push(fsWatch(f, rebuild));
346350
} catch {
347351
// file may not exist yet — that's fine
348352
}
349353
}
354+
350355
// Also watch the directories that contain source files so newly-added
351-
// files trigger rebuilds.
352-
fsWatch("src", { recursive: true }, rebuild);
356+
// files trigger rebuilds. fs.watch({ recursive: true }) is not supported
357+
// on all platforms (notably Linux), where it throws — fall back to a
358+
// non-recursive watch on the src directory in that case. The per-file
359+
// watchers above still cover every file in the build list either way.
360+
try {
361+
watchers.push(fsWatch("src", { recursive: true }, rebuild));
362+
} catch {
363+
try {
364+
watchers.push(fsWatch("src", rebuild));
365+
} catch {
366+
// src may not be watchable — per-file watchers still apply
367+
}
368+
}
369+
370+
return watchers;
353371
}
354372

355373
const args = process.argv.slice(2);

‎dist/msrcrypto.js‎

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -42,24 +42,29 @@ var operations = {};
4242

4343
operations.register = function(operationType, algorithmName, functionToCall) {
4444

45-
if (!operations[operationType]) {
45+
if (!Object.prototype.hasOwnProperty.call(operations, operationType) ||
46+
typeof operations[operationType] !== "object") {
4647
operations[operationType] = {};
4748
}
4849

4950
var op = operations[operationType];
5051

51-
if (!op[algorithmName]) {
52+
if (!Object.prototype.hasOwnProperty.call(op, algorithmName)) {
5253
op[algorithmName] = functionToCall;
5354
}
5455

5556
};
5657

5758
operations.exists = function(operationType, algorithmName) {
58-
if (!operations[operationType]) {
59+
if (!Object.prototype.hasOwnProperty.call(operations, operationType) ||
60+
typeof operations[operationType] !== "object") {
5961
return false;
6062
}
6163

62-
return operations[operationType][algorithmName] ? true : false;
64+
var op = operations[operationType];
65+
66+
return Object.prototype.hasOwnProperty.call(op, algorithmName) &&
67+
typeof op[algorithmName] === "function";
6368
};
6469

6570
var scriptUrl = (function() {
@@ -69,7 +74,7 @@ var scriptUrl = (function() {
6974
throw new Error();
7075
} catch (e) {
7176
if (e.stack) {
72-
var match = /\w+:\/\/(.+?\/)*.+\.js/.exec(e.stack);
77+
var match = /\w+:\/\/(?:[^/\s]+\/)*[^/\s]*\.js/.exec(e.stack);
7378
return (match && match.length > 0) ? match[0] : null;
7479
}
7580
}
@@ -849,11 +854,24 @@ var msrcryptoWorker = (function() {
849854
operationSubType = e.data.operationSubType;
850855

851856
var operation = e.data.operationType,
857+
algorithmName = e.data.algorithm.name,
852858
result,
853-
func = operations[operation][e.data.algorithm.name],
854859
p = e.data;
855860

856-
if (!operations.exists(operation, e.data.algorithm.name)) {
861+
if (!operations.hasOwnProperty(operation)) {
862+
throw new Error("unregistered algorithm.");
863+
}
864+
865+
var algorithmMap = operations[operation];
866+
867+
if (typeof algorithmMap !== "object" || algorithmMap === null ||
868+
!algorithmMap.hasOwnProperty(algorithmName)) {
869+
throw new Error("unregistered algorithm.");
870+
}
871+
872+
var func = algorithmMap[algorithmName];
873+
874+
if (typeof func !== "function") {
857875
throw new Error("unregistered algorithm.");
858876
}
859877

‎dist/msrcrypto.min.js‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/global.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ var scriptUrl = (function() {
2525
throw new Error();
2626
} catch (e) {
2727
if (e.stack) {
28-
var match = /\w+:\/\/(.+?\/)*.+\.js/.exec(e.stack);
28+
var match = /\w+:\/\/(?:[^/\s]+\/)*[^/\s]*\.js/.exec(e.stack);
2929
return (match && match.length > 0) ? match[0] : null;
3030
}
3131
}

‎src/operations.js‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,22 +20,27 @@ var operations = {};
2020

2121
operations.register = function(operationType, algorithmName, functionToCall) {
2222

23-
if (!operations[operationType]) {
23+
if (!Object.prototype.hasOwnProperty.call(operations, operationType) ||
24+
typeof operations[operationType] !== "object") {
2425
operations[operationType] = {};
2526
}
2627

2728
var op = operations[operationType];
2829

29-
if (!op[algorithmName]) {
30+
if (!Object.prototype.hasOwnProperty.call(op, algorithmName)) {
3031
op[algorithmName] = functionToCall;
3132
}
3233

3334
};
3435

3536
operations.exists = function(operationType, algorithmName) {
36-
if (!operations[operationType]) {
37+
if (!Object.prototype.hasOwnProperty.call(operations, operationType) ||
38+
typeof operations[operationType] !== "object") {
3739
return false;
3840
}
3941

40-
return operations[operationType][algorithmName] ? true : false;
42+
var op = operations[operationType];
43+
44+
return Object.prototype.hasOwnProperty.call(op, algorithmName) &&
45+
typeof op[algorithmName] === "function";
4146
};

‎src/worker.js‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,33 @@ var msrcryptoWorker = (function() {
4242
operationSubType = e.data.operationSubType;
4343

4444
var operation = e.data.operationType,
45+
algorithmName = e.data.algorithm.name,
4546
result,
46-
func = operations[operation][e.data.algorithm.name],
4747
p = e.data;
4848

49-
if (!operations.exists(operation, e.data.algorithm.name)) {
49+
// Resolve the registered handler with the dynamic lookup, the
50+
// own-property checks, and the function-type check all performed
51+
// locally, using the direct obj.hasOwnProperty(name) form shown in
52+
// CodeQL's "unvalidated dynamic method call" guidance. The registry
53+
// of registered operations is itself the whitelist: a user-supplied
54+
// operation/algorithm name is only honored when it is an own,
55+
// registered property — so it can never dispatch to an inherited
56+
// Object.prototype member (valueOf, hasOwnProperty, ...) or to a
57+
// non-function value.
58+
if (!operations.hasOwnProperty(operation)) {
59+
throw new Error("unregistered algorithm.");
60+
}
61+
62+
var algorithmMap = operations[operation];
63+
64+
if (typeof algorithmMap !== "object" || algorithmMap === null ||
65+
!algorithmMap.hasOwnProperty(algorithmName)) {
66+
throw new Error("unregistered algorithm.");
67+
}
68+
69+
var func = algorithmMap[algorithmName];
70+
71+
if (typeof func !== "function") {
5072
throw new Error("unregistered algorithm.");
5173
}
5274

‎test/Test.Shared.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ function slowTest() {
4343

4444
}
4545

46-
// Microsoft Edges does not export key_ops or alg properties
46+
// Microsoft Edge does not export key_ops or alg properties
4747
// unless keyImport included those properties
4848
// So, generateKey will be missing key_ops & alg
4949
// This will check if this is happening
@@ -59,7 +59,7 @@ function slowTest() {
5959

6060
var UseNative = false;
6161
var useWebWorkers = false;
62-
var iterations = 1;
62+
var iterations = 10;
6363
var skipSlowTests = true;
6464
var subtle = (UseNative && nativeCrypto) ? crypto.subtle : msrCrypto.subtle;
6565
var label = UseNative ? "(native)" : useWebWorkers ? "msrCrypto (workers)" : "msrCrypto";

0 commit comments

Comments
 (0)