Skip to content

Commit 6018296

Browse files
authored
fix(security): prevent prototype pollution in DOM helpers (#523)
* fix(security): prevent prototype pollution in DOM helpers
1 parent 615499f commit 6018296

11 files changed

Lines changed: 450 additions & 269 deletions

File tree

.github/workflows/main.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,9 @@ jobs:
5858
- name: Build Library
5959
run: pnpm build:lib
6060

61+
- name: Run Prototype Pollution Security Tests
62+
run: pnpm test:security
63+
6164
- name: Build Website (GitHub demo site)
6265
run: pnpm build:demo
6366

package.json

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
"new-publish": "lerna publish from-package",
4646
"roll-new-release": "pnpm build && pnpm new-version && pnpm new-publish",
4747
"serve:demo": "pnpm -r --stream --filter=\"{packages/demo/**}\" dev",
48+
"test:security": "pnpm -r --stream --filter=\"{packages/multiple-select-vanilla/**}\" test:security",
4849
"test:e2e": "remove playwright-report && playwright test --config playwright/playwright.config.ts",
4950
"test:e2e:debug": "playwright test --config playwright/playwright.config.ts --ui --debug",
5051
"test:e2e:ui": "playwright test --config playwright/playwright.config.ts --ui",
@@ -56,13 +57,13 @@
5657
"pnpm": "11.x"
5758
},
5859
"devDependencies": {
59-
"@biomejs/biome": "^2.5.7",
60-
"@lerna-lite/cli": "^5.4.2",
61-
"@lerna-lite/publish": "^5.4.2",
62-
"@lerna-lite/watch": "^5.4.2",
60+
"@biomejs/biome": "^2.5.9",
61+
"@lerna-lite/cli": "^5.6.1",
62+
"@lerna-lite/publish": "^5.6.1",
63+
"@lerna-lite/watch": "^5.6.1",
6364
"@playwright/test": "^1.62.1",
64-
"@types/node": "^26.1.2",
65-
"conventional-changelog-conventionalcommits": "^10.2.1",
65+
"@types/node": "^26.2.0",
66+
"conventional-changelog-conventionalcommits": "^10.4.0",
6667
"cross-env": "catalog:",
6768
"npm-run-all2": "^9.0.3",
6869
"remove-glob": "catalog:",

packages/demo/src/main.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import 'font-awesome/css/font-awesome.css';
55
import { createDomElement, emptyElement } from 'multiple-select-vanilla';
66

77
import { exampleRouting, navbarRouting } from './app-routing.js';
8-
// biome-ignore lint/correctness/useImportExtensions: false positive
98
import mainHtml from './main.html?raw';
109
import './style.scss';
1110

packages/multiple-select-vanilla/.npmignore

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,5 @@ tsconfig.tsbuildinfo
33
node_modules
44
build-prod.mjs
55
build-watch.mjs
6-
CHANGELOG.md
6+
CHANGELOG.md
7+
test

packages/multiple-select-vanilla/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,8 @@
6060
"sass:build": "sass src/styles:dist/styles/css --style=compressed --quiet-deps --no-source-map && pnpm sass:build:closing",
6161
"sass:build:closing": "postcss dist/styles/css/**/* --dir dist/styles/css --base dist/styles/css --no-map --use cssnano --use autoprefixer --style=compressed",
6262
"sass:watch": "sass src/styles:dist/styles/css --watch --style=compressed --quiet-deps --no-source-map",
63-
"sass:copy": "copyfiles \"./src/styles/**/*.scss\" dist/styles/sass --up 2 --stat"
63+
"sass:copy": "copyfiles \"./src/styles/**/*.scss\" dist/styles/sass --up 2 --stat",
64+
"test:security": "pnpm build:all && node --test test/*.test.mjs"
6465
},
6566
"dependencies": {
6667
"@types/trusted-types": "^2.0.7"

packages/multiple-select-vanilla/src/MultipleSelectInstance.ts

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ export class MultipleSelectInstance {
7979
protected elm: HTMLInputElement | HTMLSelectElement | HTMLSpanElement,
8080
options?: Partial<Omit<MultipleSelectOption, 'onHardDestroy' | 'onAfterHardDestroy'>>,
8181
) {
82-
this.options = Object.assign({}, Constants.DEFAULTS, this.elm.dataset, options) as MultipleSelectOption;
82+
this.options = { ...Constants.DEFAULTS, ...this.elm.dataset, ...options } as MultipleSelectOption;
8383
this._bindEventService = new BindingEventService({ distinctEvent: true });
8484
}
8585

@@ -132,28 +132,25 @@ export class MultipleSelectInstance {
132132
protected initLocale() {
133133
if (this.options.locale) {
134134
if (typeof this.options.locale === 'object') {
135-
Object.assign(this.options, this.options.locale);
135+
this.options = { ...this.options, ...this.options.locale };
136136
return;
137137
}
138138

139-
// Use locales from options, fallback to instance property
140-
const locales = (this.options.locales || this.locales || {}) as Record<string, MultipleSelectLocale>;
139+
// Use locales from own options only, fallback to instance property
140+
const optionLocales = Object.prototype.hasOwnProperty.call(this.options, 'locales') ? this.options.locales : undefined;
141+
const locales = (optionLocales || this.locales || {}) as Record<string, MultipleSelectLocale>;
141142
const parts = this.options.locale.split(/-|_/);
142143

143144
parts[0] = parts[0].toLowerCase();
144145
if (parts[1]) {
145146
parts[1] = parts[1].toUpperCase();
146147
}
147148

148-
if (locales[this.options.locale]) {
149-
Object.assign(this.options, locales[this.options.locale]);
150-
} else if (locales[parts.join('-')]) {
151-
Object.assign(this.options, locales[parts.join('-')]);
152-
} else if (locales[parts[0]]) {
153-
Object.assign(this.options, locales[parts[0]]);
154-
} else {
149+
const localeKey = [this.options.locale, parts.join('-'), parts[0]].find(key => Object.prototype.hasOwnProperty.call(locales, key));
150+
if (!localeKey) {
155151
throw new Error(`[multiple-select-vanilla] invalid locales "${this.options.locale}", make sure to import it before using it`);
156152
}
153+
this.options = { ...this.options, ...locales[localeKey] };
157154
}
158155
}
159156

@@ -1664,7 +1661,7 @@ export class MultipleSelectInstance {
16641661
*/
16651662
getOptions(returnDeepCopy = true) {
16661663
// deep copy and remove data
1667-
const options = Object.assign({}, this.options);
1664+
const options = { ...this.options };
16681665
delete options.data;
16691666

16701667
return returnDeepCopy ? deepCopy<MultipleSelectOption>(options) : this.options;
@@ -1675,7 +1672,7 @@ export class MultipleSelectInstance {
16751672
if (compareObjects(this.options, options, true)) {
16761673
return;
16771674
}
1678-
this.options = Object.assign(this.options, options);
1675+
this.options = { ...this.options, ...options };
16791676
this.destroy(false);
16801677
this.init();
16811678
}

packages/multiple-select-vanilla/src/services/virtual-scroll.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ export class VirtualScroll {
2727
this.parentEl = options.contentEl?.parentElement;
2828
this.callback = options.callback;
2929

30-
this.cache = {} as VirtualCache;
30+
this.cache = Object.create(null) as VirtualCache;
3131
this.scrollTop = this.scrollEl.scrollTop;
3232

3333
this.initDOM(this.rows);
@@ -51,7 +51,7 @@ export class VirtualScroll {
5151

5252
reset(rows: HtmlStruct[]) {
5353
this.lastCluster = 0;
54-
this.cache = {} as any;
54+
this.cache = Object.create(null) as VirtualCache;
5555
emptyElement(this.contentEl);
5656
this.initDOM(rows);
5757
}

packages/multiple-select-vanilla/src/utils/domUtils.ts

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,16 @@
11
import type { HtmlStruct, InferDOMType } from '../models/interfaces.js';
22
import { isDefined, objectRemoveEmptyProps } from './utils.js';
33

4+
const UNSAFE_DOM_PROPERTY_NAMES = new Set(['__proto__', 'prototype', 'constructor', 'toString', 'valueOf', 'hasOwnProperty']);
5+
6+
function assertSafeDomProperties(properties: object) {
7+
for (const propertyName of Object.keys(properties)) {
8+
if (UNSAFE_DOM_PROPERTY_NAMES.has(propertyName)) {
9+
throw new TypeError(`[multiple-select-vanilla] unsafe DOM property name "${propertyName}"`);
10+
}
11+
}
12+
}
13+
414
export interface HtmlElementPosition {
515
top: number;
616
bottom: number;
@@ -51,10 +61,13 @@ export function createDomElement<T extends keyof HTMLElementTagNameMap, K extend
5161
const elm = document.createElement<T>(tagName);
5262

5363
if (elementOptions) {
64+
assertSafeDomProperties(elementOptions);
5465
Object.keys(elementOptions).forEach(elmOptionKey => {
5566
const elmValue = elementOptions[elmOptionKey as keyof typeof elementOptions];
56-
if (typeof elmValue === 'object') {
57-
Object.assign(elm[elmOptionKey as K] as object, elmValue);
67+
const elmTarget = elm[elmOptionKey as K];
68+
if (typeof elmValue === 'object' && elmValue !== null && typeof elmTarget === 'object' && elmTarget !== null) {
69+
assertSafeDomProperties(elmValue);
70+
Object.assign(elmTarget, elmValue);
5871
} else {
5972
elm[elmOptionKey as K] = (elementOptions as any)[elmOptionKey as keyof typeof elementOptions];
6073
}
@@ -104,7 +117,7 @@ export function createDomStructure(item: HtmlStruct, appendToElm?: HTMLElement,
104117

105118
/** takes an html block object and converts to a real HTMLElement */
106119
export function convertItemRowToHtml(item: HtmlStruct): HTMLElement {
107-
if (item.hasOwnProperty('tagName')) {
120+
if (Object.prototype.hasOwnProperty.call(item, 'tagName')) {
108121
return createDomStructure(item);
109122
}
110123
return document.createElement('li');
Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
import assert from 'node:assert/strict';
2+
import { after, test } from 'node:test';
3+
4+
import { MultipleSelectInstance, VirtualScroll, convertItemRowToHtml, createDomElement } from '../dist/index.js';
5+
6+
const originalDocument = globalThis.document;
7+
8+
after(() => {
9+
globalThis.document = originalDocument;
10+
});
11+
12+
test('createDomElement rejects prototype and inherited built-in property names', () => {
13+
const elementPrototype = {};
14+
globalThis.document = {
15+
createElement: () => Object.create(elementPrototype),
16+
};
17+
18+
for (const propertyName of ['__proto__', 'prototype', 'constructor', 'toString', 'valueOf', 'hasOwnProperty']) {
19+
const properties = { [propertyName]: { polluted: propertyName } };
20+
assert.throws(() => createDomElement('div', properties), /unsafe DOM property name/);
21+
}
22+
23+
assert.equal(elementPrototype.polluted, undefined);
24+
assert.equal(Object.polluted, undefined);
25+
assert.equal(Object.prototype.toString.polluted, undefined);
26+
assert.equal(Object.prototype.valueOf.polluted, undefined);
27+
assert.equal(Object.prototype.hasOwnProperty.polluted, undefined);
28+
});
29+
30+
test('createDomElement still assigns ordinary and nested DOM properties', () => {
31+
const style = {};
32+
const dataset = {};
33+
globalThis.document = {
34+
createElement: () => ({ style, dataset }),
35+
};
36+
37+
const element = createDomElement('div', {
38+
className: 'safe-class',
39+
dataset: { key: 'safe-key' },
40+
style: { display: 'none' },
41+
});
42+
43+
assert.equal(element.className, 'safe-class');
44+
assert.deepEqual(element.dataset, { key: 'safe-key' });
45+
assert.deepEqual(element.style, { display: 'none' });
46+
});
47+
48+
test('createDomElement does not merge objects into inherited functions', () => {
49+
const addEventListener = () => {};
50+
const elementPrototype = { addEventListener };
51+
globalThis.document = {
52+
createElement: () => Object.create(elementPrototype),
53+
};
54+
55+
const payload = { polluted: true };
56+
const element = createDomElement('div', { addEventListener: payload });
57+
58+
assert.equal(addEventListener.polluted, undefined);
59+
assert.equal(Object.hasOwn(element, 'addEventListener'), true);
60+
assert.equal(element.addEventListener, payload);
61+
});
62+
63+
test('convertItemRowToHtml supports prototype-free and overridden input objects', () => {
64+
globalThis.document = {
65+
createElement: tagName => ({ tagName, appendChild: () => {}, setAttribute: () => {} }),
66+
};
67+
68+
const prototypeFreeItem = Object.assign(Object.create(null), { tagName: 'div', props: {} });
69+
const overriddenItem = { tagName: 'span', props: {}, hasOwnProperty: null };
70+
71+
assert.equal(convertItemRowToHtml(prototypeFreeItem).tagName, 'div');
72+
assert.equal(convertItemRowToHtml(overriddenItem).tagName, 'span');
73+
});
74+
75+
test('VirtualScroll uses a prototype-free cache across resets', () => {
76+
const createElement = tagName => ({ tagName, appendChild: () => {}, setAttribute: () => {}, offsetHeight: 10 });
77+
globalThis.document = { createElement };
78+
79+
const children = [];
80+
const contentElement = {
81+
children,
82+
parentElement: null,
83+
appendChild: child => children.push(child),
84+
removeChild: child => children.splice(children.indexOf(child), 1),
85+
get firstChild() {
86+
return children[0];
87+
},
88+
get lastChild() {
89+
return children.at(-1);
90+
},
91+
};
92+
const scrollElement = {
93+
scrollTop: 0,
94+
addEventListener: () => {},
95+
removeEventListener: () => {},
96+
};
97+
const rows = [{ tagName: 'li', props: { className: 'row' } }];
98+
99+
const virtualScroll = new VirtualScroll({ rows, scrollEl: scrollElement, contentEl: contentElement, callback: () => {} });
100+
assert.equal(Object.getPrototypeOf(virtualScroll.cache), null);
101+
102+
virtualScroll.reset(rows);
103+
assert.equal(Object.getPrototypeOf(virtualScroll.cache), null);
104+
});
105+
106+
test('options treat special names as own data without changing their prototype', () => {
107+
const maliciousOptions = JSON.parse('{"__proto__":{"polluted":true},"constructor":{"polluted":true},"toString":{"polluted":true}}');
108+
const instance = new MultipleSelectInstance({ dataset: {} }, maliciousOptions);
109+
const options = instance.getOptions(false);
110+
111+
assert.equal(Object.getPrototypeOf(options), Object.prototype);
112+
assert.equal(
113+
Object.getOwnPropertyDescriptor(options, '__proto__')?.value,
114+
Object.getOwnPropertyDescriptor(maliciousOptions, '__proto__')?.value,
115+
);
116+
assert.equal(Object.prototype.polluted, undefined);
117+
assert.equal(Object.polluted, undefined);
118+
assert.equal(Object.prototype.toString.polluted, undefined);
119+
});
120+
121+
test('refreshOptions cannot replace the options prototype', () => {
122+
const instance = new MultipleSelectInstance({ dataset: {} });
123+
instance.destroy = () => {};
124+
instance.init = () => {};
125+
126+
const maliciousOptions = JSON.parse('{"__proto__":{"polluted":true}}');
127+
instance.refreshOptions(maliciousOptions);
128+
const options = instance.getOptions(false);
129+
130+
assert.equal(Object.getPrototypeOf(options), Object.prototype);
131+
assert.equal(
132+
Object.getOwnPropertyDescriptor(options, '__proto__')?.value,
133+
Object.getOwnPropertyDescriptor(maliciousOptions, '__proto__')?.value,
134+
);
135+
assert.equal(Object.prototype.polluted, undefined);
136+
});
137+
138+
test('locale lookup ignores inherited option and registry properties', () => {
139+
const localeName = 'polluted-locale';
140+
const pollutedLocale = {
141+
formatSelectAll: () => 'polluted',
142+
};
143+
Object.prototype.locales = { [localeName]: pollutedLocale };
144+
145+
try {
146+
const instance = new MultipleSelectInstance({ dataset: {} }, { locale: localeName });
147+
assert.throws(() => instance.initLocale(), /invalid locales/);
148+
} finally {
149+
delete Object.prototype.locales;
150+
}
151+
152+
const instance = new MultipleSelectInstance({ dataset: {} }, { locale: localeName });
153+
instance.locales = Object.create({ [localeName]: pollutedLocale });
154+
assert.throws(() => instance.initLocale(), /invalid locales/);
155+
});

0 commit comments

Comments
 (0)