Skip to content

Commit 54b8e29

Browse files
committed
fix: protoype pollution!
1 parent 0126cda commit 54b8e29

4 files changed

Lines changed: 76 additions & 2 deletions

File tree

src/handlers/record/record-transition.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { setValue as setPathValue } from '../../utils/json-path'
1+
import { setValue as setPathValue, isValidPath } from '../../utils/json-path'
22
import RecordHandler from './record-handler'
33
import { recordRequest } from './record-request'
44
import { RecordWriteMessage, TOPIC, RECORD_ACTION, Message } from '../../constants'
@@ -128,6 +128,14 @@ export class RecordTransition {
128128
return
129129
}
130130

131+
if (message.path && !isValidPath(message.path)) {
132+
socketWrapper.sendMessage({ ...message,
133+
action: RECORD_ACTION.INVALID_MESSAGE_DATA,
134+
originalAction: message.action,
135+
})
136+
return
137+
}
138+
131139
if (message.action === RECORD_ACTION.UPDATE) {
132140
if (!isOfType(message.parsedData, 'object') && !isOfType(message.parsedData, 'array')) {
133141
socketWrapper.sendMessage({ ...message,

src/services/permission/valve/rule-application.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,11 @@ export default class RuleApplication {
231231

232232
if (typeof currentData !== UNDEFINED && currentData !== LOADING) {
233233
data = JSON.parse(JSON.stringify(currentData))
234-
setValue(data, msg.path, msg.parsedData)
234+
try {
235+
setValue(data, msg.path, msg.parsedData)
236+
} catch (e) {
237+
return e as Error
238+
}
235239
return data
236240
}
237241
this.loadRecord(this.params.name)

src/utils/json-path.spec.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,3 +328,46 @@ describe('objects are created from paths and their value is set correctly', () =
328328
})
329329

330330
})
331+
332+
describe('rejects prototype-pollution paths', () => {
333+
afterEach(() => {
334+
delete (Object.prototype as any).polluted
335+
})
336+
337+
it('throws on __proto__ as the first segment', () => {
338+
const record: any = {}
339+
expect(() => jsonPath.setValue(record, '__proto__.polluted', true)).to.throw(/forbidden/)
340+
expect(({} as any).polluted).to.equal(undefined)
341+
})
342+
343+
it('throws on __proto__ as a nested segment', () => {
344+
const record: any = { a: {} }
345+
expect(() => jsonPath.setValue(record, 'a.__proto__.polluted', true)).to.throw(/forbidden/)
346+
expect(({} as any).polluted).to.equal(undefined)
347+
})
348+
349+
it('throws on constructor.prototype path', () => {
350+
const record: any = {}
351+
expect(() => jsonPath.setValue(record, 'constructor.prototype.polluted', true)).to.throw(/forbidden/)
352+
expect(({} as any).polluted).to.equal(undefined)
353+
})
354+
355+
it('throws on a single __proto__ segment (record prototype switch)', () => {
356+
const record: any = {}
357+
expect(() => jsonPath.setValue(record, '__proto__', { polluted: true })).to.throw(/forbidden/)
358+
expect((record as any).polluted).to.equal(undefined)
359+
})
360+
361+
it('getValue refuses to traverse __proto__', () => {
362+
expect(() => jsonPath.getValue({}, '__proto__.toString')).to.throw(/forbidden/)
363+
})
364+
365+
it('isValidPath flags forbidden keys but accepts normal paths', () => {
366+
expect(jsonPath.isValidPath('__proto__.x')).to.equal(false)
367+
expect(jsonPath.isValidPath('constructor.prototype.x')).to.equal(false)
368+
expect(jsonPath.isValidPath('a.__proto__.x')).to.equal(false)
369+
expect(jsonPath.isValidPath('a.b.c')).to.equal(true)
370+
expect(jsonPath.isValidPath('items[0].name')).to.equal(true)
371+
expect(jsonPath.isValidPath('user_proto.foo')).to.equal(true)
372+
})
373+
})

src/utils/json-path.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
const SPLIT_REG_EXP = /[[\]]/g
2+
const FORBIDDEN_KEYS = new Set(['__proto__', 'constructor', 'prototype'])
23

34
/**
45
* Returns the value of the path or
@@ -49,6 +50,20 @@ export function setValue (root: any, path: string, value: any): void {
4950
}
5051
}
5152

53+
/**
54+
* Returns true if the path is safe to use with getValue / setValue.
55+
* Rejects paths containing __proto__, constructor, or prototype tokens
56+
* that would otherwise allow prototype-chain traversal.
57+
*/
58+
export function isValidPath (path: string): boolean {
59+
try {
60+
tokenize(path)
61+
return true
62+
} catch {
63+
return false
64+
}
65+
}
66+
5267
/**
5368
* Parses the path. Splits it into
5469
* keys for objects and indices for arrays.
@@ -72,6 +87,10 @@ function tokenize (path: string): Array<string | number> {
7287
continue
7388
}
7489

90+
if (FORBIDDEN_KEYS.has(arrayIndexes[0])) {
91+
throw new Error(`invalid path: forbidden key '${arrayIndexes[0]}'`)
92+
}
93+
7594
tokens.push(arrayIndexes[0])
7695

7796
for (let j = 1; j < arrayIndexes.length; j++) {

0 commit comments

Comments
 (0)