Skip to content

Commit 9b25642

Browse files
fix: prevent stack overflow in JSON merge patch generation
This commit addresses a security vulnerability where deeply nested or cyclic JSON objects could cause a stack overflow in `generateMergePatch` and `applyMergePatch`. - Introduces a `MAX_DEPTH` limit of 1000 for recursion in `src/core/json-utils.ts`. - Updates `generateMergePatch` and `applyMergePatch` to throw an error if the depth limit is exceeded. - Adds comprehensive tests in `tests/unit/json_utils_security.test.ts` to verify the fix and ensure no regressions. This change ensures the extension is robust against malicious or accidentally deep JSON structures. Co-authored-by: zknpr <96851588+zknpr@users.noreply.github.com>
1 parent e9c4c24 commit 9b25642

2 files changed

Lines changed: 70 additions & 4 deletions

File tree

src/core/json-utils.ts

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,13 @@
77
* SQLite's json_patch works this way.
88
*/
99

10-
export function generateMergePatch(original: any, modified: any): any {
10+
const MAX_DEPTH = 1000;
11+
12+
export function generateMergePatch(original: any, modified: any, depth = 0): any {
13+
if (depth > MAX_DEPTH) {
14+
throw new Error('JSON merge patch depth limit exceeded');
15+
}
16+
1117
if (original === modified) {
1218
return undefined; // No change
1319
}
@@ -35,7 +41,7 @@ export function generateMergePatch(original: any, modified: any): any {
3541
hasChanges = true;
3642
} else if (originalVal !== modifiedVal) {
3743
// Modification
38-
const subPatch = generateMergePatch(originalVal, modifiedVal);
44+
const subPatch = generateMergePatch(originalVal, modifiedVal, depth + 1);
3945
if (subPatch !== undefined) {
4046
patch[key] = subPatch;
4147
hasChanges = true;
@@ -61,7 +67,11 @@ export function generateMergePatch(original: any, modified: any): any {
6167
* @param patch - The patch to apply
6268
* @returns The modified object (new instance or mutated)
6369
*/
64-
export function applyMergePatch(target: any, patch: any): any {
70+
export function applyMergePatch(target: any, patch: any, depth = 0): any {
71+
if (depth > MAX_DEPTH) {
72+
throw new Error('JSON apply merge patch depth limit exceeded');
73+
}
74+
6575
if (patch === null) {
6676
// If patch is null, it typically means deletion in a parent context,
6777
// but at the root level, it means the result is null.
@@ -87,7 +97,7 @@ export function applyMergePatch(target: any, patch: any): any {
8797
if (val === null) {
8898
delete target[key];
8999
} else {
90-
target[key] = applyMergePatch(target[key], val);
100+
target[key] = applyMergePatch(target[key], val, depth + 1);
91101
}
92102
}
93103

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
2+
import { describe, it } from 'node:test';
3+
import assert from 'node:assert';
4+
import { generateMergePatch, applyMergePatch } from '../../src/core/json-utils';
5+
6+
describe('JSON Merge Patch Security', () => {
7+
it('generateMergePatch should throw on deep recursion', () => {
8+
const depth = 1100; // > 1000
9+
let original: any = { a: 1 };
10+
let modified: any = { a: 2 };
11+
12+
for (let i = 0; i < depth; i++) {
13+
original = { next: original };
14+
modified = { next: modified };
15+
}
16+
17+
try {
18+
generateMergePatch(original, modified);
19+
assert.fail('Should have thrown depth limit error');
20+
} catch (e: any) {
21+
assert.match(e.message, /JSON merge patch depth limit exceeded/);
22+
}
23+
});
24+
25+
it('applyMergePatch should throw on deep recursion', () => {
26+
const depth = 1100; // > 1000
27+
let target: any = { a: 1 };
28+
let patch: any = { a: 2 };
29+
30+
for (let i = 0; i < depth; i++) {
31+
target = { next: target };
32+
patch = { next: patch };
33+
}
34+
35+
try {
36+
applyMergePatch(target, patch);
37+
assert.fail('Should have thrown depth limit error');
38+
} catch (e: any) {
39+
assert.match(e.message, /JSON apply merge patch depth limit exceeded/);
40+
}
41+
});
42+
43+
it('should handle cyclic references by depth limit', () => {
44+
const original: any = { a: 1 };
45+
original.self = original;
46+
const modified: any = { a: 2 };
47+
modified.self = modified;
48+
49+
try {
50+
generateMergePatch(original, modified);
51+
assert.fail('Should have thrown depth limit error');
52+
} catch (e: any) {
53+
assert.match(e.message, /JSON merge patch depth limit exceeded/);
54+
}
55+
});
56+
});

0 commit comments

Comments
 (0)