Skip to content

Commit f607fdc

Browse files
dcojgithub-actions[bot]
authored andcommitted
Validate import api for potential prototype pollution
GitOrigin-RevId: 326092937e4e1e8a163c1d2a7ae660ea6692a21d
1 parent 8d131ca commit f607fdc

5 files changed

Lines changed: 64 additions & 12 deletions

File tree

src/style-spec/validate/validate_import.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,11 @@ export default function validateImport(options: ImportValidatorOptions): Validat
4343
errors.push(new ValidationError(key, importSpec, `import id can't be an empty string`));
4444
}
4545

46-
// Reject reserved prototype-pollution key — the import id is used as a scope
47-
// string that becomes a dictionary key in several runtime caches.
48-
if (unbundle(importSpec.id) === '__proto__') {
46+
// Reject reserved prototype-pollution keys
47+
const importId = unbundle(importSpec.id);
48+
if (importId === '__proto__' || importId === 'constructor' || importId === 'prototype') {
4949
const key = `${options.key}.id`;
50-
errors.push(new ValidationError(key, importSpec, `import id can't be "__proto__"`));
50+
errors.push(new ValidationError(key, importSpec, `import id can't be "${String(importId)}"`));
5151
}
5252

5353
if (data) {

src/style/style.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4634,6 +4634,12 @@ class Style extends Evented<MapEvents> {
46344634
addImport(importSpec: ImportSpecification, beforeId?: string | null): Promise<void> {
46354635
this._checkLoaded();
46364636

4637+
const reservedImportIds = new Set(['__proto__', 'constructor', 'prototype']);
4638+
if (reservedImportIds.has(importSpec.id)) {
4639+
this.fire(new ErrorEvent(new Error(`Import id can't be "${importSpec.id}".`)));
4640+
return;
4641+
}
4642+
46374643
const imports = this.stylesheet.imports = this.stylesheet.imports || [];
46384644

46394645
const index = imports.findIndex(({id}) => id === importSpec.id);

src/style/style_changes.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,13 @@ class StyleChanges {
2727
constructor() {
2828
this._changed = false;
2929

30-
this._updatedLayers = {};
31-
this._removedLayers = {};
30+
this._updatedLayers = Object.create(null) as typeof this._updatedLayers;
31+
this._removedLayers = Object.create(null) as typeof this._removedLayers;
3232

33-
this._updatedSourceCaches = {};
33+
this._updatedSourceCaches = Object.create(null) as typeof this._updatedSourceCaches;
3434
this._updatedPaintProps = new Set();
3535

36-
this._updatedImages = {};
36+
this._updatedImages = Object.create(null) as typeof this._updatedImages;
3737
}
3838

3939
isDirty(): boolean {
@@ -173,13 +173,13 @@ class StyleChanges {
173173
reset() {
174174
this._changed = false;
175175

176-
this._updatedLayers = {};
177-
this._removedLayers = {};
176+
this._updatedLayers = Object.create(null) as typeof this._updatedLayers;
177+
this._removedLayers = Object.create(null) as typeof this._removedLayers;
178178

179-
this._updatedSourceCaches = {};
179+
this._updatedSourceCaches = Object.create(null) as typeof this._updatedSourceCaches;
180180
this._updatedPaintProps.clear();
181181

182-
this._updatedImages = {};
182+
this._updatedImages = Object.create(null) as typeof this._updatedImages;
183183
}
184184
}
185185

src/ui/map.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2467,6 +2467,12 @@ export class Map extends Camera {
24672467
return false;
24682468
}
24692469

2470+
// Disallow reserved prototype property names
2471+
if (id === '__proto__' || id === 'constructor' || id === 'prototype') {
2472+
this.fire(new ErrorEvent(new Error(`IDs can't be "${id}".`)));
2473+
return false;
2474+
}
2475+
24702476
return true;
24712477
}
24722478

@@ -3175,6 +3181,10 @@ export class Map extends Camera {
31753181
* }, 'basemap');
31763182
*/
31773183
addImport(importSpecification: ImportSpecification, beforeId?: string | null): this {
3184+
if (!this._isValidId(importSpecification.id)) {
3185+
return this;
3186+
}
3187+
31783188
this.style.addImport(importSpecification, beforeId)
31793189
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
31803190
.catch((e) => this.fire(new ErrorEvent(new Error('Failed to add import', e))));

test/unit/style/style_imports.test.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -779,6 +779,42 @@ describe('Style#addImport', () => {
779779
'streets-v2'
780780
]);
781781
});
782+
783+
test('rejects "__proto__" import id to prevent prototype pollution', async () => {
784+
const {style} = newStubStyle();
785+
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
786+
style.loadJSON(createStyleJSON());
787+
await waitFor(style, 'style.load');
788+
789+
const errorSpy = vi.fn();
790+
style.on('error', errorSpy);
791+
792+
// addImport fires an error event and returns early without mutating state
793+
style.addImport({id: '__proto__', url: '/style.json'});
794+
795+
expect(errorSpy).toHaveBeenCalledOnce();
796+
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
797+
expect(errorSpy.mock.calls[0][0].error.message).toMatch('__proto__');
798+
// Must not have added the import and must not have polluted Object.prototype
799+
expect(style.stylesheet.imports ?? []).toHaveLength(0);
800+
expect(Object.hasOwn({}, 0)).toBe(false);
801+
});
802+
803+
test('rejects "constructor" and "prototype" import ids to prevent prototype pollution', async () => {
804+
const {style} = newStubStyle();
805+
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
806+
style.loadJSON(createStyleJSON());
807+
await waitFor(style, 'style.load');
808+
809+
const errorSpy = vi.fn();
810+
style.on('error', errorSpy);
811+
812+
style.addImport({id: 'constructor', url: '/style.json'});
813+
style.addImport({id: 'prototype', url: '/style.json'});
814+
815+
expect(errorSpy).toHaveBeenCalledTimes(2);
816+
expect(style.stylesheet.imports ?? []).toHaveLength(0);
817+
});
782818
});
783819

784820
describe('Style#updateImport', () => {

0 commit comments

Comments
 (0)