Skip to content

Commit 113dd1f

Browse files
mcollinamcollina
authored
Sanitize input so it's safer (#168)
* sanitize * sanitize json pointer * correct defaults for props * correctly escape regexp * Correct sanitize multiple slashes * last round of feedbacks
1 parent a19c836 commit 113dd1f

File tree

3 files changed

+181
-20
lines changed

3 files changed

+181
-20
lines changed

‎index.js

Lines changed: 38 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
var Ajv = require('ajv')
44
var merge = require('deepmerge')
5+
var util = require('util')
56
var validate = require('./schema-validator')
67

78
var uglify = null
@@ -310,7 +311,7 @@ function addPatternProperties (schema, externalSchema, fullSchema) {
310311
}
311312
var type = pp[regex].type
312313
code += `
313-
if (/${regex}/.test(keys[i])) {
314+
if (/${regex.replace(/\\*\//g, '\\/')}/.test(keys[i])) {
314315
`
315316
if (type === 'object') {
316317
code += buildObject(pp[regex], '', 'buildObjectPP' + index, externalSchema, fullSchema)
@@ -505,13 +506,27 @@ function refFinder (ref, schema, externalSchema) {
505506
return dereferenced
506507
} else {
507508
for (var i = 1; i < walk.length; i++) {
508-
code += `['${walk[i]}']`
509+
code += `['${sanitizeKey(walk[i])}']`
509510
}
510511
}
511512
}
512513
return (new Function('schema', code))(schema)
513514
}
514515

516+
function sanitizeKey (key) {
517+
const rep = key.replace(/(\\*)'/g, function (match, p1) {
518+
var base = ''
519+
if (p1.length % 2 === 1) {
520+
base = p1.slice(2)
521+
} else {
522+
base = p1
523+
}
524+
var rep = base + '\\\''
525+
return rep
526+
})
527+
return rep
528+
}
529+
515530
function buildCode (schema, code, laterCode, name, externalSchema, fullSchema) {
516531
Object.keys(schema.properties || {}).forEach((key, i, a) => {
517532
if (schema.properties[key]['$ref']) {
@@ -523,49 +538,51 @@ function buildCode (schema, code, laterCode, name, externalSchema, fullSchema) {
523538

524539
var type = schema.properties[key].type
525540
var nullable = schema.properties[key].nullable
541+
var sanitized = sanitizeKey(key)
542+
var asString = sanitizeKey($asString(key).replace(/\\/g, '\\\\'))
526543

527544
if (nullable) {
528545
code += `
529-
if (obj['${key}'] === null) {
546+
if (obj['${sanitized}'] === null) {
530547
${addComma}
531-
json += '${$asString(key)}:null'
548+
json += '${asString}:null'
532549
var rendered = true
533550
} else {
534551
`
535552
}
536553

537554
if (type === 'number') {
538555
code += `
539-
var t = Number(obj['${key}'])
556+
var t = Number(obj['${sanitized}'])
540557
if (!isNaN(t)) {
541558
${addComma}
542-
json += '${$asString(key)}:' + t
559+
json += '${asString}:' + t
543560
`
544561
} else if (type === 'integer') {
545562
code += `
546563
var rendered = false
547564
`
548565
if (isLong) {
549566
code += `
550-
if (isLong(obj['${key}'])) {
567+
if (isLong(obj['${sanitized}'])) {
551568
${addComma}
552-
json += '${$asString(key)}:' + obj['${key}'].toString()
569+
json += '${asString}:' + obj['${sanitized}'].toString()
553570
rendered = true
554571
} else {
555-
var t = Number(obj['${key}'])
572+
var t = Number(obj['${sanitized}'])
556573
if (!isNaN(t)) {
557574
${addComma}
558-
json += '${$asString(key)}:' + t
575+
json += '${asString}:' + t
559576
rendered = true
560577
}
561578
}
562579
`
563580
} else {
564581
code += `
565-
var t = Number(obj['${key}'])
582+
var t = Number(obj['${sanitized}'])
566583
if (!isNaN(t)) {
567584
${addComma}
568-
json += '${$asString(key)}:' + t
585+
json += '${asString}:' + t
569586
rendered = true
570587
}
571588
`
@@ -575,9 +592,9 @@ function buildCode (schema, code, laterCode, name, externalSchema, fullSchema) {
575592
`
576593
} else {
577594
code += `
578-
if (obj['${key}'] !== undefined) {
595+
if (obj['${sanitized}'] !== undefined) {
579596
${addComma}
580-
json += '${$asString(key)}:'
597+
json += '${asString}:'
581598
`
582599

583600
var result = nested(laterCode, name, key, schema.properties[key], externalSchema, fullSchema)
@@ -590,12 +607,12 @@ function buildCode (schema, code, laterCode, name, externalSchema, fullSchema) {
590607
code += `
591608
} else {
592609
${addComma}
593-
json += '${$asString(key)}:${JSON.stringify(defaultValue).replace(/'/g, '\'')}'
610+
json += '${asString}:${sanitizeKey(JSON.stringify(defaultValue).replace(/\\/g, '\\\\'))}'
594611
`
595612
} else if (schema.required && schema.required.indexOf(key) !== -1) {
596613
code += `
597614
} else {
598-
throw new Error('${key} is required!')
615+
throw new Error('${sanitized} is required!')
599616
`
600617
}
601618

@@ -658,7 +675,7 @@ function addIfThenElse (schema, name, externalSchema, fullSchema) {
658675
var merged = merge(copy, then)
659676

660677
code += `
661-
valid = ajv.validate(${require('util').inspect(i, { depth: null })}, obj)
678+
valid = ajv.validate(${util.inspect(i, { depth: null })}, obj)
662679
if (valid) {
663680
`
664681
if (merged.if && merged.then) {
@@ -856,6 +873,8 @@ function nested (laterCode, name, key, schema, externalSchema, fullSchema, subKe
856873
var code = ''
857874
var funcName
858875

876+
subKey = subKey || ''
877+
859878
if (schema.type === undefined) {
860879
var inferedType = inferTypeByKeyword(schema)
861880
if (inferedType) {
@@ -866,7 +885,7 @@ function nested (laterCode, name, key, schema, externalSchema, fullSchema, subKe
866885
var type = schema.type
867886
var nullable = schema.nullable === true
868887

869-
var accessor = key.indexOf('[') === 0 ? key : `['${key}']`
888+
var accessor = key.indexOf('[') === 0 ? sanitizeKey(key) : `['${sanitizeKey(key)}']`
870889
switch (type) {
871890
case 'null':
872891
code += `
@@ -902,7 +921,7 @@ function nested (laterCode, name, key, schema, externalSchema, fullSchema, subKe
902921
case undefined:
903922
if ('anyOf' in schema) {
904923
schema.anyOf.forEach((s, index) => {
905-
var nestedResult = nested(laterCode, name, key, s, externalSchema, fullSchema, subKey !== undefined ? subKey : 'i' + index)
924+
var nestedResult = nested(laterCode, name, key, s, externalSchema, fullSchema, subKey !== '' ? subKey : 'i' + index)
906925
code += `
907926
${index === 0 ? 'if' : 'else if'}(ajv.validate(${require('util').inspect(s, { depth: null })}, obj${accessor}))
908927
${nestedResult.code}

‎test/anyof.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
'use strict'
22

3-
const test = require('tap').test
3+
const { test } = require('tap')
44
const build = require('..')
55

66
test('object with multiple types field', (t) => {

‎test/sanitize.test.js

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
'use strict'
2+
3+
const t = require('tap')
4+
const build = require('..')
5+
6+
const stringify = build({
7+
title: 'Example Schema',
8+
type: 'object',
9+
properties: {
10+
firstName: {
11+
type: 'string'
12+
},
13+
lastName: {
14+
type: 'string'
15+
},
16+
age: {
17+
description: 'Age in years"',
18+
type: 'integer'
19+
},
20+
[(() => `phra'&& process.exit(1) ||'phra`)()]: {},
21+
now: {
22+
type: 'string'
23+
},
24+
reg: {
25+
type: 'string',
26+
default: 'a\'&& process.exit(1) ||\''
27+
},
28+
obj: {
29+
type: 'object',
30+
properties: {
31+
bool: {
32+
type: 'boolean'
33+
}
34+
}
35+
},
36+
'"\'w00t': {
37+
type: 'string',
38+
default: '"\'w00t'
39+
},
40+
arr: {
41+
type: 'array',
42+
items: {
43+
type: 'object',
44+
properties: {
45+
'phra\' && process.exit(1)//': {
46+
type: 'number'
47+
},
48+
str: {
49+
type: 'string'
50+
}
51+
}
52+
}
53+
}
54+
},
55+
required: ['now'],
56+
patternProperties: {
57+
'.*foo$': {
58+
type: 'string'
59+
},
60+
'test': {
61+
type: 'number'
62+
},
63+
'phra\'/ && process.exit(1) && /\'': {
64+
type: 'number'
65+
},
66+
'"\'w00t.*////': {
67+
type: 'number'
68+
}
69+
},
70+
additionalProperties: {
71+
type: 'string'
72+
}
73+
})
74+
75+
const obj = {
76+
firstName: 'Matteo',
77+
lastName: 'Collina',
78+
age: 32,
79+
now: new Date(),
80+
foo: 'hello"',
81+
bar: "world'",
82+
'fuzz"': 42,
83+
"me'": 42,
84+
numfoo: 42,
85+
test: 42,
86+
strtest: '23',
87+
arr: [{ 'phra\' && process.exit(1)//': 42 }],
88+
obj: { bool: true },
89+
notmatch: 'valar morghulis',
90+
notmatchobj: { a: true },
91+
notmatchnum: 42
92+
}
93+
94+
// pass if it does not crash
95+
const json = stringify(obj)
96+
JSON.parse(json)
97+
98+
const stringify2 = build({
99+
title: 'Example Schema',
100+
type: 'object',
101+
patternProperties: {
102+
'"\'w00t.*////': {
103+
type: 'number'
104+
}
105+
}
106+
})
107+
108+
t.deepEqual(JSON.parse(stringify2({
109+
'"\'phra////': 42,
110+
'asd': 42
111+
})), {
112+
})
113+
114+
const stringify3 = build({
115+
title: 'Example Schema',
116+
type: 'object',
117+
properties: {
118+
"\"phra\\'&&(console.log(42))//||'phra": {}
119+
}
120+
})
121+
122+
// this verifies the escaping
123+
JSON.parse(stringify3({
124+
'"phra\'&&(console.log(42))//||\'phra': 42
125+
}))
126+
127+
const stringify4 = build({
128+
title: 'Example Schema',
129+
type: 'object',
130+
properties: {
131+
'"\\\\\\\\\'w00t': {
132+
type: 'string',
133+
default: '"\'w00t'
134+
}
135+
}
136+
})
137+
138+
t.deepEqual(JSON.parse(stringify4({})), {
139+
'"\\\\\\\\\'w00t': '"\'w00t'
140+
})
141+
142+
t.pass('no crashes')

0 commit comments

Comments
 (0)