Skip to content

Commit 0d54b0c

Browse files
wellwelwelwellwelwel
authoredMar 26, 2024··
fix(cache): improve cache key serialization (#2424)
* fix(cache): improve cache key formation. Fixes a potential parser cache poisoning attack vulnerability reported by Vsevolod Kokorin (Slonser) of Solidlab
·
v3.14.0v3.9.3
1 parent d9dccfd commit 0d54b0c

File tree

2 files changed

+518
-15
lines changed

2 files changed

+518
-15
lines changed
 

‎lib/parsers/parser_cache.js

+28-15
Original file line numberDiff line numberDiff line change
@@ -3,26 +3,38 @@
33
const LRU = require('lru-cache').default;
44

55
const parserCache = new LRU({
6-
max: 15000
6+
max: 15000,
77
});
88

99
function keyFromFields(type, fields, options, config) {
10-
let res =
11-
`${type}` +
12-
`/${typeof options.nestTables}` +
13-
`/${options.nestTables}` +
14-
`/${options.rowsAsArray}` +
15-
`/${options.supportBigNumbers || config.supportBigNumbers}` +
16-
`/${options.bigNumberStrings || config.bigNumberStrings}` +
17-
`/${typeof options.typeCast}` +
18-
`/${options.timezone || config.timezone}` +
19-
`/${options.decimalNumbers}` +
20-
`/${options.dateStrings}`;
10+
const res = [
11+
type,
12+
typeof options.nestTables,
13+
options.nestTables,
14+
Boolean(options.rowsAsArray),
15+
Boolean(options.supportBigNumbers || config.supportBigNumbers),
16+
Boolean(options.bigNumberStrings || config.bigNumberStrings),
17+
typeof options.typeCast,
18+
options.timezone || config.timezone,
19+
Boolean(options.decimalNumbers),
20+
options.dateStrings,
21+
];
22+
2123
for (let i = 0; i < fields.length; ++i) {
2224
const field = fields[i];
23-
res += `/${field.name}:${field.columnType}:${field.length}:${field.schema}:${field.table}:${field.flags}:${field.characterSet}`;
25+
26+
res.push([
27+
field.name,
28+
field.columnType,
29+
field.length,
30+
field.schema,
31+
field.table,
32+
field.flags,
33+
field.characterSet,
34+
]);
2435
}
25-
return res;
36+
37+
return JSON.stringify(res, null, 0);
2638
}
2739

2840
function getParser(type, fields, options, config, compiler) {
@@ -49,5 +61,6 @@ function clearCache() {
4961
module.exports = {
5062
getParser: getParser,
5163
setMaxCache: setMaxCache,
52-
clearCache: clearCache
64+
clearCache: clearCache,
65+
_keyFromFields: keyFromFields,
5366
};

‎test/unit/parsers/cache-key-serialization.js

+490
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,490 @@
1+
'use strict';
2+
3+
const assert = require('assert');
4+
const _keyFromFields =
5+
require('../../../lib/parsers/parser_cache.js')._keyFromFields;
6+
7+
// Invalid
8+
const test1 = {
9+
type: undefined,
10+
fields: [
11+
{
12+
name: undefined,
13+
columnType: undefined,
14+
length: undefined,
15+
schema: undefined,
16+
table: undefined,
17+
flags: undefined,
18+
characterSet: undefined,
19+
},
20+
],
21+
options: {
22+
nestTables: undefined,
23+
rowsAsArray: undefined,
24+
supportBigNumbers: undefined,
25+
bigNumberStrings: undefined,
26+
typeCast: undefined,
27+
timezone: undefined,
28+
decimalNumbers: undefined,
29+
dateStrings: undefined,
30+
},
31+
config: {
32+
supportBigNumbers: undefined,
33+
bigNumberStrings: undefined,
34+
timezone: undefined,
35+
},
36+
};
37+
38+
// Invalid, except for `config` (global overwriting)
39+
const test2 = {
40+
type: undefined,
41+
fields: [
42+
{
43+
name: undefined,
44+
columnType: undefined,
45+
length: undefined,
46+
schema: undefined,
47+
table: undefined,
48+
flags: undefined,
49+
characterSet: undefined,
50+
},
51+
],
52+
options: {
53+
nestTables: undefined,
54+
rowsAsArray: undefined,
55+
supportBigNumbers: undefined,
56+
bigNumberStrings: undefined,
57+
typeCast: undefined,
58+
timezone: undefined,
59+
decimalNumbers: undefined,
60+
dateStrings: undefined,
61+
},
62+
config: {
63+
supportBigNumbers: false,
64+
bigNumberStrings: false,
65+
timezone: 'local',
66+
},
67+
};
68+
69+
// Invalid, except for options
70+
const test3 = {
71+
type: undefined,
72+
fields: [
73+
{
74+
name: undefined,
75+
columnType: undefined,
76+
length: undefined,
77+
schema: undefined,
78+
table: undefined,
79+
flags: undefined,
80+
characterSet: undefined,
81+
},
82+
],
83+
options: {
84+
nestTables: '',
85+
rowsAsArray: false,
86+
supportBigNumbers: false,
87+
bigNumberStrings: false,
88+
typeCast: true,
89+
timezone: 'local',
90+
decimalNumbers: false,
91+
dateStrings: false,
92+
},
93+
config: {
94+
supportBigNumbers: undefined,
95+
bigNumberStrings: undefined,
96+
timezone: undefined,
97+
},
98+
};
99+
100+
// Based on results of `SELECT * FROM test WHERE value = ?`
101+
const test4 = {
102+
type: 'binary',
103+
fields: [
104+
{
105+
name: 'id',
106+
columnType: '3',
107+
length: undefined,
108+
schema: 'test',
109+
table: 'test',
110+
flags: '16899',
111+
characterSet: '63',
112+
},
113+
{
114+
name: 'value',
115+
columnType: '246',
116+
length: undefined,
117+
schema: 'test',
118+
table: 'test',
119+
flags: '0',
120+
characterSet: '63',
121+
},
122+
],
123+
options: {
124+
nestTables: false,
125+
rowsAsArray: false,
126+
supportBigNumbers: false,
127+
bigNumberStrings: false,
128+
typeCast: true,
129+
timezone: 'local',
130+
decimalNumbers: false,
131+
dateStrings: 'DATETIME',
132+
},
133+
config: {
134+
supportBigNumbers: undefined,
135+
bigNumberStrings: undefined,
136+
timezone: undefined,
137+
},
138+
};
139+
140+
// Same from test4, but with invalid booleans need to reach out the same key
141+
const test5 = {
142+
type: 'binary',
143+
fields: [
144+
{
145+
name: 'id',
146+
columnType: '3',
147+
length: undefined,
148+
schema: 'test',
149+
table: 'test',
150+
flags: '16899',
151+
characterSet: '63',
152+
},
153+
{
154+
name: 'value',
155+
columnType: '246',
156+
length: undefined,
157+
schema: 'test',
158+
table: 'test',
159+
flags: '0',
160+
characterSet: '63',
161+
},
162+
],
163+
options: {
164+
nestTables: false,
165+
rowsAsArray: undefined,
166+
supportBigNumbers: undefined,
167+
bigNumberStrings: undefined,
168+
typeCast: true,
169+
timezone: 'local',
170+
decimalNumbers: undefined,
171+
dateStrings: 'DATETIME',
172+
},
173+
config: {
174+
supportBigNumbers: undefined,
175+
bigNumberStrings: undefined,
176+
timezone: undefined,
177+
},
178+
};
179+
180+
// Forcing delimiters on strings fields
181+
// Checking for quotes escape
182+
const test6 = {
183+
type: 'binary',
184+
fields: [
185+
{
186+
name: ':',
187+
columnType: '©',
188+
length: undefined,
189+
schema: '/',
190+
table: ',',
191+
flags: '_',
192+
characterSet: '❌',
193+
},
194+
],
195+
options: {
196+
nestTables: false,
197+
rowsAsArray: true,
198+
supportBigNumbers: true,
199+
bigNumberStrings: true,
200+
typeCast: true,
201+
timezone: '""`\'',
202+
decimalNumbers: true,
203+
dateStrings: '#',
204+
},
205+
config: {
206+
supportBigNumbers: undefined,
207+
bigNumberStrings: undefined,
208+
timezone: undefined,
209+
},
210+
};
211+
212+
// valid with `true` on booleans
213+
const test7 = {
214+
type: 'binary',
215+
fields: [
216+
{
217+
name: 'id',
218+
columnType: '3',
219+
length: undefined,
220+
schema: 'test',
221+
table: 'test',
222+
flags: '16899',
223+
characterSet: '63',
224+
},
225+
{
226+
name: 'value',
227+
columnType: '246',
228+
length: undefined,
229+
schema: 'test',
230+
table: 'test',
231+
flags: '0',
232+
characterSet: '63',
233+
},
234+
],
235+
options: {
236+
nestTables: true,
237+
rowsAsArray: true,
238+
supportBigNumbers: true,
239+
bigNumberStrings: true,
240+
typeCast: true,
241+
timezone: 'local',
242+
decimalNumbers: true,
243+
dateStrings: 'DATETIME',
244+
},
245+
config: {
246+
supportBigNumbers: true,
247+
bigNumberStrings: true,
248+
timezone: true,
249+
},
250+
};
251+
252+
// Expects the same result from test7, but using valid values instead of `true` on booleans fields
253+
const test8 = {
254+
type: 'binary',
255+
fields: [
256+
{
257+
name: 'id',
258+
columnType: '3',
259+
length: undefined,
260+
schema: 'test',
261+
table: 'test',
262+
flags: '16899',
263+
characterSet: '63',
264+
},
265+
{
266+
name: 'value',
267+
columnType: '246',
268+
length: undefined,
269+
schema: 'test',
270+
table: 'test',
271+
flags: '0',
272+
characterSet: '63',
273+
},
274+
],
275+
options: {
276+
nestTables: true,
277+
rowsAsArray: 2,
278+
supportBigNumbers: 'yes',
279+
bigNumberStrings: [],
280+
typeCast: true,
281+
timezone: 'local',
282+
decimalNumbers: {
283+
a: null,
284+
},
285+
dateStrings: 'DATETIME',
286+
},
287+
config: {
288+
supportBigNumbers: true,
289+
bigNumberStrings: true,
290+
timezone: true,
291+
},
292+
};
293+
294+
// Invalid: checking function parser in wrong fields, expecting to be `null`
295+
const test9 = {
296+
type: 'binary',
297+
fields: [
298+
{
299+
name: 'id',
300+
columnType: '3',
301+
length: undefined,
302+
schema: 'test',
303+
table: 'test',
304+
flags: '16899',
305+
characterSet: '63',
306+
},
307+
],
308+
options: {
309+
nestTables: false,
310+
rowsAsArray: false,
311+
supportBigNumbers: false,
312+
// Expected: true
313+
bigNumberStrings: (_, next) => next(),
314+
// Expected: "function"
315+
typeCast: (_, next) => next(),
316+
timezone: 'local',
317+
decimalNumbers: false,
318+
// Expected: null
319+
dateStrings: (_, next) => next(),
320+
},
321+
config: {
322+
supportBigNumbers: undefined,
323+
bigNumberStrings: undefined,
324+
timezone: undefined,
325+
},
326+
};
327+
328+
const result1 = _keyFromFields(
329+
test1.type,
330+
test1.fields,
331+
test1.options,
332+
test1.config,
333+
);
334+
const result2 = _keyFromFields(
335+
test2.type,
336+
test2.fields,
337+
test2.options,
338+
test2.config,
339+
);
340+
const result3 = _keyFromFields(
341+
test3.type,
342+
test3.fields,
343+
test3.options,
344+
test3.config,
345+
);
346+
const result4 = _keyFromFields(
347+
test4.type,
348+
test4.fields,
349+
test4.options,
350+
test4.config,
351+
);
352+
const result5 = _keyFromFields(
353+
test5.type,
354+
test5.fields,
355+
test5.options,
356+
test5.config,
357+
);
358+
const result6 = _keyFromFields(
359+
test6.type,
360+
test6.fields,
361+
test6.options,
362+
test6.config,
363+
);
364+
const result7 = _keyFromFields(
365+
test7.type,
366+
test7.fields,
367+
test7.options,
368+
test7.config,
369+
);
370+
const result8 = _keyFromFields(
371+
test8.type,
372+
test8.fields,
373+
test8.options,
374+
test8.config,
375+
);
376+
const result9 = _keyFromFields(
377+
test9.type,
378+
test9.fields,
379+
test9.options,
380+
test9.config,
381+
);
382+
383+
assert.deepStrictEqual(
384+
result1,
385+
'[null,"undefined",null,false,false,false,"undefined",null,false,null,[null,null,null,null,null,null,null]]',
386+
);
387+
assert(JSON.parse(result1));
388+
389+
assert.deepStrictEqual(
390+
result2,
391+
'[null,"undefined",null,false,false,false,"undefined","local",false,null,[null,null,null,null,null,null,null]]',
392+
);
393+
assert(JSON.parse(result2));
394+
395+
assert.deepStrictEqual(
396+
result3,
397+
'[null,"string","",false,false,false,"boolean","local",false,false,[null,null,null,null,null,null,null]]',
398+
);
399+
assert(JSON.parse(result3));
400+
401+
assert.deepStrictEqual(
402+
result4,
403+
'["binary","boolean",false,false,false,false,"boolean","local",false,"DATETIME",["id","3",null,"test","test","16899","63"],["value","246",null,"test","test","0","63"]]',
404+
);
405+
assert(JSON.parse(result4));
406+
407+
assert.deepStrictEqual(result4, result5);
408+
assert(JSON.parse(result5));
409+
410+
assert.deepStrictEqual(
411+
result6,
412+
'["binary","boolean",false,true,true,true,"boolean","\\"\\"`\'",true,"#",[":","©",null,"/",",","_","❌"]]',
413+
);
414+
// Ensuring that JSON is valid with invalid delimiters
415+
assert(JSON.parse(result6));
416+
417+
assert.deepStrictEqual(
418+
result7,
419+
'["binary","boolean",true,true,true,true,"boolean","local",true,"DATETIME",["id","3",null,"test","test","16899","63"],["value","246",null,"test","test","0","63"]]',
420+
);
421+
assert(JSON.parse(result7));
422+
423+
assert.deepStrictEqual(result7, result8);
424+
assert(JSON.parse(result8));
425+
426+
assert.deepStrictEqual(
427+
result9,
428+
'["binary","boolean",false,false,false,true,"function","local",false,null,["id","3",null,"test","test","16899","63"]]',
429+
);
430+
assert(JSON.parse(result9));
431+
assert(JSON.parse(result9)[5] === true);
432+
assert(JSON.parse(result9)[6] === 'function');
433+
assert(JSON.parse(result9)[9] === null);
434+
435+
// Testing twice all existent tests needs to return 7 keys, since two of them expects to be the same
436+
assert(
437+
Array.from(
438+
new Set([
439+
_keyFromFields(test1.type, test1.fields, test1.options, test1.config),
440+
_keyFromFields(test1.type, test1.fields, test1.options, test1.config),
441+
_keyFromFields(test2.type, test2.fields, test2.options, test2.config),
442+
_keyFromFields(test2.type, test2.fields, test2.options, test2.config),
443+
_keyFromFields(test3.type, test3.fields, test3.options, test3.config),
444+
_keyFromFields(test3.type, test3.fields, test3.options, test3.config),
445+
_keyFromFields(test4.type, test4.fields, test4.options, test4.config),
446+
_keyFromFields(test4.type, test4.fields, test4.options, test4.config),
447+
_keyFromFields(test5.type, test5.fields, test5.options, test5.config),
448+
_keyFromFields(test5.type, test5.fields, test5.options, test5.config),
449+
_keyFromFields(test6.type, test6.fields, test6.options, test6.config),
450+
_keyFromFields(test6.type, test6.fields, test6.options, test6.config),
451+
_keyFromFields(test7.type, test7.fields, test7.options, test7.config),
452+
_keyFromFields(test7.type, test7.fields, test7.options, test7.config),
453+
_keyFromFields(test8.type, test8.fields, test8.options, test8.config),
454+
_keyFromFields(test8.type, test8.fields, test8.options, test8.config),
455+
_keyFromFields(test9.type, test9.fields, test9.options, test9.config),
456+
_keyFromFields(test9.type, test9.fields, test9.options, test9.config),
457+
]),
458+
).length === 7,
459+
);
460+
461+
const stringify = JSON.stringify;
462+
463+
// Overwriting the native `JSON.stringify`
464+
JSON.stringify = (value, replacer, space = 8) => stringify(value, replacer, space);
465+
466+
// Testing twice all existent tests needs to return 7 keys, since two of them expects to be the same
467+
assert(
468+
Array.from(
469+
new Set([
470+
result1,
471+
_keyFromFields(test1.type, test1.fields, test1.options, test1.config),
472+
result2,
473+
_keyFromFields(test2.type, test2.fields, test2.options, test2.config),
474+
result3,
475+
_keyFromFields(test3.type, test3.fields, test3.options, test3.config),
476+
result4,
477+
_keyFromFields(test4.type, test4.fields, test4.options, test4.config),
478+
result5,
479+
_keyFromFields(test5.type, test5.fields, test5.options, test5.config),
480+
result6,
481+
_keyFromFields(test6.type, test6.fields, test6.options, test6.config),
482+
result7,
483+
_keyFromFields(test7.type, test7.fields, test7.options, test7.config),
484+
result8,
485+
_keyFromFields(test8.type, test8.fields, test8.options, test8.config),
486+
result9,
487+
_keyFromFields(test9.type, test9.fields, test9.options, test9.config),
488+
]),
489+
).length === 7,
490+
);

0 commit comments

Comments
 (0)
Please sign in to comment.