Skip to content

Commit 91175b3

Browse files
cxcorpHarelM
andauthored
Fix validate_object crashing when object prototype keys used in style's objects (#1028)
* Fix validation crashing for specific key names in objects Fixes an unhandled error occurring when validated objects (e.g. sources) had keys that were found in Object.prototype. This caused certain truthyness checks to incorrectly pass, as e.g. object["__proto__"] always returns a truthy value (unless object has a null prototype or otherwise non-Object prototype). This commit fixes the problem by checking that the value is indeed set on the object via Object.hasOwn(), or Object.prototype.hasOwnProperty.call() if hasOwn is not available. * Update CHANGELOG * Update CHANGELOG.md * Add unit tests for has_own.ts * Make hasOwn check for truthiness for reusability, add getOwn util hasOwn can now be used to replace patterns like: ``` if (hasOwnProperty(obj, key) && obj[key]) {} ``` with ``` if (hasOwn(obj, key)) {} ``` whereas getOwn can be used to replace patterns like ``` const value = (hasOwnProperty(obj, key) && obj[key]) || fallback ``` with ``` const value = getOwn(obj, key) || fallback ``` * Remove hasOwn utility in favor of just using getOwn --------- Co-authored-by: Harel M <[email protected]>
1 parent 786044f commit 91175b3

File tree

6 files changed

+148
-3
lines changed

6 files changed

+148
-3
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
### 🐞 Bug fixes
77
- Fix RuntimeError class, make it inherited from Error ([#983](https://github.com/maplibre/maplibre-style-spec/issues/983))
8+
- Fix `validate_object` crashing when object prototype keys used in style's objects ([#1028](https://github.com/maplibre/maplibre-style-spec/pull/1028))
89
- Validate that `layers` array items are objects instead of throwing an error if not ([!1026](https://github.com/maplibre/maplibre-style-spec/pull/1026))
910

1011
## 23.1.0

src/util/get_own.test.ts

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
describe('get_own', () => {
2+
describe.each([
3+
[
4+
'when Object.hasOwn is available',
5+
function beforeAllFn() {
6+
// clear require cache before running tests as the implementation of
7+
// hasOwn depends on whether Object.hasOwn is available
8+
jest.resetModules();
9+
expect(Object.hasOwn).toBeInstanceOf(Function);
10+
},
11+
],
12+
[
13+
'when Object.hasOwn is not available',
14+
function beforeAllFn() {
15+
jest.resetModules();
16+
delete Object.hasOwn;
17+
expect(Object.hasOwn).toBeUndefined();
18+
},
19+
],
20+
])('unit tests %s', (_, beforeAllFn) => {
21+
let getOwn: typeof import('./get_own').getOwn;
22+
23+
beforeAll(async () => {
24+
beforeAllFn();
25+
26+
const res = await import('./get_own');
27+
getOwn = res.getOwn;
28+
});
29+
30+
afterEach(() => {
31+
jest.resetModules();
32+
});
33+
34+
test('returns value for own properties', () => {
35+
const obj = {key: 'value'};
36+
expect(getOwn(obj, 'key')).toBe('value');
37+
});
38+
39+
test('returns value for falsy own properties', () => {
40+
const obj = {key: false, key2: 0, key3: '', key4: undefined, key5: null};
41+
expect(getOwn(obj, 'key')).toBe(false);
42+
expect(getOwn(obj, 'key2')).toBe(0);
43+
expect(getOwn(obj, 'key3')).toBe('');
44+
expect(getOwn(obj, 'key4')).toBeUndefined();
45+
expect(getOwn(obj, 'key5')).toBeNull();
46+
});
47+
48+
test('returns undefined for properties inherited from the prototype', () => {
49+
const obj = {key: 'value'};
50+
expect(getOwn(obj, '__proto__')).toBeUndefined();
51+
expect(getOwn(obj, 'constructor')).toBeUndefined();
52+
expect(getOwn(obj, 'valueOf')).toBeUndefined();
53+
54+
const inheritedKey = 'inheritedKey';
55+
const prototype = {[inheritedKey]: 1234};
56+
const objWithPrototype = Object.create(prototype);
57+
expect(getOwn(objWithPrototype, inheritedKey)).toBeUndefined();
58+
});
59+
60+
test('returns true for own properties that have the same name as a property in the prototype', () => {
61+
const obj = JSON.parse('{"__proto__": 123, "valueOf": "123"}');
62+
expect(getOwn(obj, '__proto__')).toBe(123);
63+
expect(getOwn(obj, 'valueOf')).toBe('123');
64+
});
65+
});
66+
});

src/util/get_own.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
type HasOwnPropertyFn = <TObject extends object>(obj: TObject, key: PropertyKey) => key is keyof TObject;
2+
3+
// polyfill for Object.hasOwn
4+
const hasOwnProperty: HasOwnPropertyFn =
5+
(Object.hasOwn as HasOwnPropertyFn) ||
6+
function hasOwnProperty<T extends object>(object: T, key: PropertyKey): key is keyof T {
7+
return Object.prototype.hasOwnProperty.call(object, key);
8+
};
9+
10+
export function getOwn<T extends object>(object: T, key: PropertyKey): T[keyof T] | undefined {
11+
return hasOwnProperty(object, key) ? object[key] : undefined;
12+
}
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
import {validate} from './validate';
2+
import {validateObject} from './validate_object';
3+
import v8 from '../reference/v8.json' with {type: 'json'};
4+
5+
describe('Validate object', () => {
6+
test('Should not throw an unexpected error if object prototype keys are used as keys', () => {
7+
const errors = validateObject({
8+
key: 'test',
9+
value: {
10+
__defineProperty__: 123,
11+
hasOwnProperty: 123,
12+
toLocaleString: 123,
13+
valueOf: 123,
14+
},
15+
style: {},
16+
styleSpec: v8,
17+
validateSpec: validate,
18+
});
19+
expect(errors).toEqual([
20+
{message: 'test: unknown property "__defineProperty__"'},
21+
{message: 'test: unknown property "hasOwnProperty"'},
22+
{message: 'test: unknown property "toLocaleString"'},
23+
{message: 'test: unknown property "valueOf"'},
24+
]);
25+
});
26+
27+
test('Should not throw an unexpected error if object prototype keys are used as dot separated keys', () => {
28+
const errors = validateObject({
29+
key: 'test',
30+
value: {
31+
'__proto__.__proto__': 123,
32+
'__defineProperty__.__defineProperty__': 123,
33+
'hasOwnProperty.hasOwnProperty': 123,
34+
'toLocaleString.toLocaleString': 123,
35+
'valueOf.valueOf': 123,
36+
},
37+
style: {},
38+
styleSpec: v8,
39+
validateSpec: validate,
40+
});
41+
expect(errors).toEqual([
42+
{message: 'test: unknown property "__proto__.__proto__"'},
43+
{message: 'test: unknown property "__defineProperty__.__defineProperty__"'},
44+
{message: 'test: unknown property "hasOwnProperty.hasOwnProperty"'},
45+
{message: 'test: unknown property "toLocaleString.toLocaleString"'},
46+
{message: 'test: unknown property "valueOf.valueOf"'},
47+
]);
48+
});
49+
});

src/validate/validate_object.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11

22
import {ValidationError} from '../error/validation_error';
3+
import {getOwn} from '../util/get_own';
34
import {getType} from '../util/get_type';
45

56
export function validateObject(options): Array<ValidationError> {
@@ -19,12 +20,13 @@ export function validateObject(options): Array<ValidationError> {
1920

2021
for (const objectKey in object) {
2122
const elementSpecKey = objectKey.split('.')[0]; // treat 'paint.*' as 'paint'
22-
const elementSpec = elementSpecs[elementSpecKey] || elementSpecs['*'];
23+
// objectKey comes from the user controlled style input, so elementSpecKey may be e.g. "__proto__"
24+
const elementSpec = getOwn(elementSpecs, elementSpecKey) || elementSpecs['*'];
2325

2426
let validateElement;
25-
if (elementValidators[elementSpecKey]) {
27+
if (getOwn(elementValidators, elementSpecKey)) {
2628
validateElement = elementValidators[elementSpecKey];
27-
} else if (elementSpecs[elementSpecKey]) {
29+
} else if (getOwn(elementSpecs, elementSpecKey)) {
2830
validateElement = validateSpec;
2931
} else if (elementValidators['*']) {
3032
validateElement = elementValidators['*'];

test/integration/style-spec/tests/sources.input.json

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,21 @@
4949
"zoom": ["+", ["zoom"]],
5050
"state": ["+", ["feature-state", "foo"]]
5151
}
52+
},
53+
"__proto__in-dot-separated-key-name-does-not-crash": {
54+
"type": "raster",
55+
"mazoom": 10,
56+
"tileSize": 256,
57+
"tiles": ["http://example.com/{x}/{y}/{z}.png"],
58+
"__proto__.__proto__": 123
59+
},
60+
"key-names-with-object-prototype-methods-do-not-crash": {
61+
"type": "raster",
62+
"mazoom": 10,
63+
"tileSize": 256,
64+
"tiles": ["http://example.com/{x}/{y}/{z}.png"],
65+
"valueOf": null,
66+
"toLocaleString": 6
5267
}
5368
},
5469
"layers": []

0 commit comments

Comments
 (0)