fix: block prototype pollution in merge-allof-schema-properties.js

[anupamme]

The mergeAllOfSchemaProperties function uses lodash's mergeWith to merge OpenAPI schema objects without validating property names. An attacker who can supply a malicious OpenAPI document with 'proto', 'constructor', or 'prototype' property names can pollute Object.prototype, affecting all JavaScript objects in the application runtime. The affected code is packages/ruleset/src/utils/merge-allof-schema-properties.js:1, and this change addresses it.

Reference: CWE-1321

What changed

• packages/ruleset/src/utils/merge-allof-schema-properties.js

Verification

No automated check could be run against this repository, so this change is unverified beyond review. Please treat it as a suggestion.

Regression test

The security boundary is maintained under adversarial input

Test

const { mergeAllOfSchemaProperties } = require('./packages/ruleset/src/utils/merge-allof-schema-properties.js');

describe("mergeAllOfSchemaProperties must not pollute Object.prototype", () => {
  const payloads = [
    { allOf: [{ __proto__: { polluted: true } }] },
    { allOf: [{ constructor: { prototype: { polluted: true } } }] },
    { allOf: [{ type: 'string' }, { description: 'valid' }] },
  ];

  beforeEach(() => {
    delete Object.prototype.polluted;
  });

  test.each(payloads)("prevents prototype pollution for input: %j", (payload) => {
    mergeAllOfSchemaProperties(payload);
    expect(Object.prototype.polluted).toBeUndefined();
  });
});

[pyrooka]

Thanks for the report — the concern is well-intentioned, but this change is not needed. The codebase is already protected against CWE-1321 (prototype pollution) through two independent layers, verified against the actual runtime.

Why the fix is redundant

Layer 1 — JSON.parse(JSON.stringify(schema)) strips dangerous keys

Before any mergeWith call, mergeAllOfSchemaProperties deep-clones the input on line 27:

const sourceSchema = JSON.parse(JSON.stringify(schema));

JSON.stringify serialises __proto__ as a plain string key (not the prototype slot), and JSON.parse creates a plain own-property named "__proto__" on a fresh object — it does not assign to Object.prototype. This alone neutralises the __proto__ vector before lodash is ever involved.

Layer 2 — lodash 4.18.1 has built-in prototype pollution protection

Lodash's internal _safeGet.js (used by _baseMergeDeep, which powers mergeWith) explicitly guards against both attack vectors:

function safeGet(object, key) {
  if (key === 'constructor' && typeof object[key] === 'function') { return; }
  if (key == '__proto__') { return; }
  return object[key];
}

This was the fix for CVE-2019-10744, shipped in lodash ≥ 4.17.12. This project uses 4.18.1, so it is covered.

Layer 3 — Live confirmation

Running the exact payloads from the PR's regression test against the current code confirms Object.prototype.polluted remains undefined in every case:

Test 1 (__proto__ via JSON.parse)  → polluted: undefined ✅
Test 2 (__proto__ literal)         → polluted: undefined ✅
Test 3 (constructor.prototype)     → polluted: undefined ✅

Summary

The proposed customizer key-guard is correct in principle, but lodash never passes dangerous keys to the customizer in the first place — _safeGet blocks them before they reach that point. Adding a redundant check would be noise without any security benefit.

Closing this PR as not needed.

[anupamme]

Thanks for taking the time to verify this. I agree that with the current Lodash version and the JSON normalisation, I haven’t demonstrated a working prototype-pollution exploit against the current implementation.

If you're open to it, I can revise the PR accordingly rather than overstate the CWE-1321 finding. My intent with the guard was to make the merge boundary explicitly reject prototype-sensitive keys instead of relying entirely on Lodash’s current protections.

I’ll add proper regression coverage using JSON-derived proto and constructor.prototype payloads and verify the behaviour against the repository’s actual test suite. If the tests show that the guard provides no additional security value beyond the existing Lodash protections, then there is no need for this PR.