From a4e91cf40401ff0c173de8e43bda5f59e95c0093 Mon Sep 17 00:00:00 2001 From: Cyril Walle Date: Tue, 10 Mar 2026 18:21:50 +0100 Subject: [PATCH 1/6] fix:import stream --- src/Formidable.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Formidable.js b/src/Formidable.js index 5ea964e0..ae12ac95 100644 --- a/src/Formidable.js +++ b/src/Formidable.js @@ -6,6 +6,7 @@ import dezalgo from 'dezalgo'; import { EventEmitter } from 'node:events'; import fsPromises from 'node:fs/promises'; import os from 'node:os'; +import stream from 'node:stream'; import path from 'node:path'; import { StringDecoder } from 'node:string_decoder'; import once from 'once'; @@ -274,7 +275,7 @@ class IncomingForm extends EventEmitter { break; default: - pipe = node_stream.Transform({ + pipe = stream.Transform({ transform: function (chunk, encoding, callback) { callback(null, chunk); } From b320639cadafe28ba33f01ea235571f59f77c118 Mon Sep 17 00:00:00 2001 From: Cyril Walle Date: Tue, 10 Mar 2026 18:21:59 +0100 Subject: [PATCH 2/6] Create prototype_contamination.test.js --- .../prototype_contamination.test.js | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 test-node/standalone/prototype_contamination.test.js diff --git a/test-node/standalone/prototype_contamination.test.js b/test-node/standalone/prototype_contamination.test.js new file mode 100644 index 00000000..c6b09fff --- /dev/null +++ b/test-node/standalone/prototype_contamination.test.js @@ -0,0 +1,58 @@ +import { ok, strictEqual } from 'node:assert'; +import { createServer } from 'node:http'; +import test from 'node:test'; +import formidable, { errors } from '../../src/index.js'; + + + +let server; +let port = 13000; + +test.beforeEach(() => { + // Increment port to avoid conflicts between tests + port += 1; + server = createServer(); +}); + +test.afterEach(() => { + return new Promise((resolve) => { + if (server.listening) { + server.close(() => resolve()); + } else { + resolve(); + } + }); +}); + +test('prototype contamination', async (t) => { + server.on('request', async (req, res) => { + const form = formidable(); + + const [fields, files] = await form.parse(req); + strictEqual(typeof String(fields), 'string', "the toString method should not be compromised"); + + res.writeHead(200); + res.end("ok"); + + }); + + await new Promise(resolve => server.listen(port, resolve)); + + const body = `{"toString":"x","hasOwnProperty":"x","a":5}`; + + const resClient = await fetch(String(new URL(`http:localhost:${port}/`)), { + method: 'POST', + headers: { + 'Content-Length': body.length, + Host: `localhost:${port}`, + 'Content-Type': 'text/json;', + }, + body + }); + + strictEqual(resClient.status, 200); + + const text = await resClient.text(); + + t.ok(true) +}); \ No newline at end of file From 4562d7a880b4b2f5f249d0482f059dd201397ad1 Mon Sep 17 00:00:00 2001 From: Cyril Walle Date: Fri, 13 Mar 2026 14:13:17 +0100 Subject: [PATCH 3/6] Update prototype_contamination.test.js --- .../prototype_contamination.test.js | 97 ++++++++++--------- 1 file changed, 53 insertions(+), 44 deletions(-) diff --git a/test-node/standalone/prototype_contamination.test.js b/test-node/standalone/prototype_contamination.test.js index c6b09fff..7fb52071 100644 --- a/test-node/standalone/prototype_contamination.test.js +++ b/test-node/standalone/prototype_contamination.test.js @@ -1,4 +1,4 @@ -import { ok, strictEqual } from 'node:assert'; +import { ok, strictEqual } from 'node:assert'; import { createServer } from 'node:http'; import test from 'node:test'; import formidable, { errors } from '../../src/index.js'; @@ -9,50 +9,59 @@ let server; let port = 13000; test.beforeEach(() => { - // Increment port to avoid conflicts between tests - port += 1; - server = createServer(); + // Increment port to avoid conflicts between tests + port += 1; + server = createServer(); }); -test.afterEach(() => { - return new Promise((resolve) => { - if (server.listening) { - server.close(() => resolve()); - } else { - resolve(); - } - }); +test('prototype contamination', async (t) => { + server.on('request', async (req, res) => { + const form = formidable(); + + const [fields, files] = await form.parse(req); + + let a; + try { + a = typeof String(fields); + } catch { + console.log("the toString method should not be compromised") + } + // strictEqual(a, 'string', "the toString method should not be compromised"); + + res.writeHead(200); + res.end("ok"); + + }); + + await new Promise(resolve => server.listen(port, resolve)); + + const body = `{"toString":"x","hasOwnProperty":"x","a":5}`; + + const resClient = await fetch(String(new URL(`http:localhost:${port}/`)), { + method: 'POST', + headers: { + 'Content-Length': body.length, + Host: `localhost:${port}`, + 'Content-Type': 'text/json;', + }, + body + }); + + strictEqual(resClient.status, 200); + + // const text = await resClient.text(); + + // t.ok(text); }); -test('prototype contamination', async (t) => { - server.on('request', async (req, res) => { - const form = formidable(); - - const [fields, files] = await form.parse(req); - strictEqual(typeof String(fields), 'string', "the toString method should not be compromised"); - - res.writeHead(200); - res.end("ok"); - - }); - - await new Promise(resolve => server.listen(port, resolve)); - - const body = `{"toString":"x","hasOwnProperty":"x","a":5}`; - - const resClient = await fetch(String(new URL(`http:localhost:${port}/`)), { - method: 'POST', - headers: { - 'Content-Length': body.length, - Host: `localhost:${port}`, - 'Content-Type': 'text/json;', - }, - body - }); - - strictEqual(resClient.status, 200); - - const text = await resClient.text(); - - t.ok(true) -}); \ No newline at end of file + + +test.afterEach(async () => { + await new Promise((resolve) => { + if (server.listening) { + server.close(() => resolve()); + } else { + resolve(); + } + }); +}); From cc203ca73d10e1435d5744890602dd0784aa4934 Mon Sep 17 00:00:00 2001 From: Cyril Walle Date: Fri, 13 Mar 2026 14:16:02 +0100 Subject: [PATCH 4/6] Update prototype_contamination.test.js --- .../standalone/prototype_contamination.test.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test-node/standalone/prototype_contamination.test.js b/test-node/standalone/prototype_contamination.test.js index 7fb52071..42a4ad09 100644 --- a/test-node/standalone/prototype_contamination.test.js +++ b/test-node/standalone/prototype_contamination.test.js @@ -20,16 +20,16 @@ test('prototype contamination', async (t) => { const [fields, files] = await form.parse(req); + res.writeHead(200); + res.end("ok"); + let a; try { a = typeof String(fields); } catch { - console.log("the toString method should not be compromised") + ; } - // strictEqual(a, 'string', "the toString method should not be compromised"); - - res.writeHead(200); - res.end("ok"); + strictEqual(a, 'string', "the toString method should not be compromised"); }); @@ -49,9 +49,9 @@ test('prototype contamination', async (t) => { strictEqual(resClient.status, 200); - // const text = await resClient.text(); + const text = await resClient.text(); - // t.ok(text); + t.ok(text); }); From 6035eb927770c338ab0c93c5a7a1a5565dc07280 Mon Sep 17 00:00:00 2001 From: Cyril Walle Date: Fri, 13 Mar 2026 14:38:14 +0100 Subject: [PATCH 5/6] fix: use empty prototype --- src/Formidable.js | 4 ++-- src/plugins/json.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Formidable.js b/src/Formidable.js index ae12ac95..ce492c1c 100644 --- a/src/Formidable.js +++ b/src/Formidable.js @@ -202,8 +202,8 @@ class IncomingForm extends EventEmitter { } } const callback = once(dezalgo(cb)); - this.fields = {}; - const files = {}; + this.fields = Object.create(null); + const files = Object.create(null); this.on('field', (name, value) => { if (this.type === 'multipart' || this.type === 'urlencoded') { diff --git a/src/plugins/json.js b/src/plugins/json.js index ca1e6f24..9503f0ac 100644 --- a/src/plugins/json.js +++ b/src/plugins/json.js @@ -27,7 +27,7 @@ function init(_self, _opts) { const parser = new JSONParser(this.options); parser.on('data', (fields) => { - this.fields = fields; + this.fields = Object.assign(Object.create(null), fields); }); parser.once('end', () => { From dbb91e723e9764f6df7a65d426383d6aee74016a Mon Sep 17 00:00:00 2001 From: Cyril Walle Date: Fri, 13 Mar 2026 14:51:18 +0100 Subject: [PATCH 6/6] Update prototype_contamination.test.js --- .../prototype_contamination.test.js | 44 ++++++++++++++++++- 1 file changed, 42 insertions(+), 2 deletions(-) diff --git a/test-node/standalone/prototype_contamination.test.js b/test-node/standalone/prototype_contamination.test.js index 42a4ad09..3efdaf71 100644 --- a/test-node/standalone/prototype_contamination.test.js +++ b/test-node/standalone/prototype_contamination.test.js @@ -29,7 +29,7 @@ test('prototype contamination', async (t) => { } catch { ; } - strictEqual(a, 'string', "the toString method should not be compromised"); + strictEqual(a, undefined, "the toString method should not be used directly"); }); @@ -51,7 +51,47 @@ test('prototype contamination', async (t) => { const text = await resClient.text(); - t.ok(text); + ok(text); +}); + +test('should not use unsafe methods on user provided objects', async (t) => { + server.on('request', async (req, res) => { + const form = formidable(); + + const [fields, files] = await form.parse(req); + + res.writeHead(200); + res.end("ok"); + + let a; + try { + a = typeof String(fields); + } catch { + ; + } + strictEqual(a, undefined, "the toString method should not be used directly"); + + }); + + await new Promise(resolve => server.listen(port, resolve)); + + const body = `{"a":"x","b":"x","z":5}`; + + const resClient = await fetch(String(new URL(`http:localhost:${port}/`)), { + method: 'POST', + headers: { + 'Content-Length': body.length, + Host: `localhost:${port}`, + 'Content-Type': 'text/json;', + }, + body + }); + + strictEqual(resClient.status, 200); + + const text = await resClient.text(); + + ok(text); });