Skip to content

Commit d77dba4

Browse files
mcollinamcollina
committed
Remove all known code executions
Not a vulnerability as schemas should be treated as code, however it's good to fix anyway. Ref: https://hackerone.com/reports/898381
1 parent 4ee784c commit d77dba4

File tree

9 files changed

+159
-66
lines changed

9 files changed

+159
-66
lines changed

‎index.js

Lines changed: 53 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,11 @@ function addPatternProperties (schema, externalSchema, fullSchema) {
378378
var type = pp[regex].type
379379
var format = pp[regex].format
380380
var stringSerializer = getStringSerializer(format)
381+
try {
382+
RegExp(regex)
383+
} catch (err) {
384+
throw new Error(`${err.message}. Found at ${regex} matching ${JSON.stringify(pp[regex])}`)
385+
}
381386
code += `
382387
if (/${regex.replace(/\\*\//g, '\\/')}/.test(keys[i])) {
383388
`
@@ -420,7 +425,7 @@ function addPatternProperties (schema, externalSchema, fullSchema) {
420425
`
421426
} else {
422427
code += `
423-
throw new Error('Cannot coerce ' + obj[keys[i]] + ' to ${type}')
428+
throw new Error('Cannot coerce ' + obj[keys[i]] + ' to ' + ${JSON.stringify(type)})
424429
`
425430
}
426431

@@ -513,7 +518,7 @@ function additionalProperty (schema, externalSchema, fullSchema) {
513518
`
514519
} else {
515520
code += `
516-
throw new Error('Cannot coerce ' + obj[keys[i]] + ' to ${type}')
521+
throw new Error('Cannot coerce ' + obj[keys[i]] + ' to ' + ${JSON.stringify(type)})
517522
`
518523
}
519524
return code
@@ -582,7 +587,7 @@ function refFinder (ref, schema, externalSchema) {
582587
return dereferenced
583588
} else {
584589
for (var i = 1; i < walk.length; i++) {
585-
code += `['${sanitizeKey(walk[i])}']`
590+
code += `[${JSON.stringify(walk[i])}]`
586591
}
587592
}
588593
}
@@ -598,34 +603,20 @@ function refFinder (ref, schema, externalSchema) {
598603

599604
function findBadKey (obj, keys) {
600605
if (keys.length === 0) return null
601-
const key = sanitizeKey(keys.shift())
606+
const key = keys.shift()
602607
if (obj[key] === undefined) {
603608
stringSimilarity = stringSimilarity || require('string-similarity')
604609
const { bestMatch } = stringSimilarity.findBestMatch(key, Object.keys(obj))
605610
if (bestMatch.rating >= 0.5) {
606-
throw new Error(`Cannot find reference '${key}', did you mean '${bestMatch.target}'?`)
611+
throw new Error(`Cannot find reference ${JSON.stringify(key)}, did you mean ${JSON.stringify(bestMatch.target)}?`)
607612
} else {
608-
throw new Error(`Cannot find reference '${key}'`)
613+
throw new Error(`Cannot find reference ${JSON.stringify(key)}`)
609614
}
610615
}
611616
return findBadKey(obj[key], keys)
612617
}
613618
}
614619

615-
function sanitizeKey (key) {
616-
const rep = key.replace(/(\\*)'/g, function (match, p1) {
617-
var base = ''
618-
if (p1.length % 2 === 1) {
619-
base = p1.slice(2)
620-
} else {
621-
base = p1
622-
}
623-
var rep = base + '\\\''
624-
return rep
625-
})
626-
return rep
627-
}
628-
629620
function buildCode (schema, code, laterCode, name, externalSchema, fullSchema) {
630621
if (schema.$ref) {
631622
schema = refFinder(schema.$ref, fullSchema, externalSchema)
@@ -645,51 +636,51 @@ function buildCode (schema, code, laterCode, name, externalSchema, fullSchema) {
645636

646637
var type = schema.properties[key].type
647638
var nullable = schema.properties[key].nullable
648-
var sanitized = sanitizeKey(key)
649-
var asString = sanitizeKey($asString(key).replace(/\\/g, '\\\\'))
639+
var sanitized = JSON.stringify(key)
640+
var asString = JSON.stringify(sanitized)
650641

651642
if (nullable) {
652643
code += `
653-
if (obj['${sanitized}'] === null) {
644+
if (obj[${sanitized}] === null) {
654645
${addComma}
655-
json += '${asString}:null'
646+
json += ${asString} + ':null'
656647
var rendered = true
657648
} else {
658649
`
659650
}
660651

661652
if (type === 'number') {
662653
code += `
663-
var t = Number(obj['${sanitized}'])
654+
var t = Number(obj[${sanitized}])
664655
if (!isNaN(t)) {
665656
${addComma}
666-
json += '${asString}:' + t
657+
json += ${asString} + ':' + t
667658
`
668659
} else if (type === 'integer') {
669660
code += `
670661
var rendered = false
671662
`
672663
if (isLong) {
673664
code += `
674-
if (isLong(obj['${sanitized}'])) {
665+
if (isLong(obj[${sanitized}])) {
675666
${addComma}
676-
json += '${asString}:' + obj['${sanitized}'].toString()
667+
json += ${asString} + ':' + obj[${sanitized}].toString()
677668
rendered = true
678669
} else {
679-
var t = Number(obj['${sanitized}'])
670+
var t = Number(obj[${sanitized}])
680671
if (!isNaN(t)) {
681672
${addComma}
682-
json += '${asString}:' + t
673+
json += ${asString} + ':' + t
683674
rendered = true
684675
}
685676
}
686677
`
687678
} else {
688679
code += `
689-
var t = Number(obj['${sanitized}'])
680+
var t = Number(obj[${sanitized}])
690681
if (!isNaN(t)) {
691682
${addComma}
692-
json += '${asString}:' + t
683+
json += ${asString} + ':' + t
693684
rendered = true
694685
}
695686
`
@@ -699,12 +690,12 @@ function buildCode (schema, code, laterCode, name, externalSchema, fullSchema) {
699690
`
700691
} else {
701692
code += `
702-
if (obj['${sanitized}'] !== undefined) {
693+
if (obj[${sanitized}] !== undefined) {
703694
${addComma}
704-
json += '${asString}:'
695+
json += ${asString} + ':'
705696
`
706697

707-
var result = nested(laterCode, name, key, schema.properties[key], externalSchema, fullSchema)
698+
var result = nested(laterCode, name, key, schema.properties[key], externalSchema, fullSchema, undefined, false)
708699
code += result.code
709700
laterCode = result.laterCode
710701
}
@@ -715,7 +706,7 @@ function buildCode (schema, code, laterCode, name, externalSchema, fullSchema) {
715706
code += `
716707
} else {
717708
${addComma}
718-
json += '${asString}:${sanitizeKey(JSON.stringify(defaultValue).replace(/\\/g, '\\\\'))}'
709+
json += ${asString} + ':' + ${JSON.stringify(JSON.stringify(defaultValue))}
719710
`
720711
} else if (schema.required && schema.required.indexOf(key) !== -1) {
721712
required = filterRequired(schema.required, key)
@@ -742,12 +733,12 @@ function buildCode (schema, code, laterCode, name, externalSchema, fullSchema) {
742733
if (i > 0) {
743734
code += ','
744735
}
745-
code += `${$asString(required[i]).replace(/\\/g, '\\\\')}`
736+
code += `${JSON.stringify(required[i])}`
746737
}
747738
code += ']'
748739
code += `
749740
for (var i = 0; i < required.length; i++) {
750-
if (obj[required[i]] === undefined) throw new Error(required[i] + ' is required!')
741+
if (obj[required[i]] === undefined) throw new Error('"' + required[i] + '" is required!')
751742
}
752743
`
753744
}
@@ -922,7 +913,7 @@ function buildArray (schema, code, name, externalSchema, fullSchema) {
922913
if (Array.isArray(schema.items)) {
923914
result = schema.items.reduce((res, item, i) => {
924915
var accessor = '[i]'
925-
const tmpRes = nested(laterCode, name, accessor, item, externalSchema, fullSchema, i)
916+
const tmpRes = nested(laterCode, name, accessor, item, externalSchema, fullSchema, i, true)
926917
var condition = `i === ${i} && ${buildArrayTypeCondition(item.type, accessor)}`
927918
return {
928919
code: `${res.code}
@@ -939,7 +930,7 @@ function buildArray (schema, code, name, externalSchema, fullSchema) {
939930
}
940931
`
941932
} else {
942-
result = nested(laterCode, name, '[i]', schema.items, externalSchema, fullSchema)
933+
result = nested(laterCode, name, '[i]', schema.items, externalSchema, fullSchema, undefined, true)
943934
}
944935

945936
code += `
@@ -1013,7 +1004,21 @@ function dereferenceOfRefs (schema, externalSchema, fullSchema, type) {
10131004
})
10141005
}
10151006

1016-
function nested (laterCode, name, key, schema, externalSchema, fullSchema, subKey) {
1007+
var strNameCounter = 0
1008+
function asFuncName (str) {
1009+
// only allow chars that can work
1010+
var rep = str.replace(/[^a-zA-Z0-9$_]/g, '')
1011+
1012+
if (rep.length === 0) {
1013+
return 'anan' + strNameCounter++
1014+
} else if (rep !== str) {
1015+
rep += strNameCounter++
1016+
}
1017+
1018+
return rep
1019+
}
1020+
1021+
function nested (laterCode, name, key, schema, externalSchema, fullSchema, subKey, isArray) {
10171022
var code = ''
10181023
var funcName
10191024

@@ -1033,7 +1038,8 @@ function nested (laterCode, name, key, schema, externalSchema, fullSchema, subKe
10331038
var type = schema.type
10341039
var nullable = schema.nullable === true
10351040

1036-
var accessor = key.indexOf('[') === 0 ? sanitizeKey(key) : `['${sanitizeKey(key)}']`
1041+
var accessor = isArray ? key : `[${JSON.stringify(key)}]`
1042+
10371043
switch (type) {
10381044
case 'null':
10391045
code += `
@@ -1054,14 +1060,14 @@ function nested (laterCode, name, key, schema, externalSchema, fullSchema, subKe
10541060
code += nullable ? `json += obj${accessor} === null ? null : $asBoolean(obj${accessor})` : `json += $asBoolean(obj${accessor})`
10551061
break
10561062
case 'object':
1057-
funcName = (name + key + subKey).replace(/[-.\[\] ]/g, '').replace(/[@]/g, 'AT_SYMBOL') // eslint-disable-line
1063+
funcName = asFuncName(name + key + subKey)
10581064
laterCode = buildObject(schema, laterCode, funcName, externalSchema, fullSchema)
10591065
code += `
10601066
json += ${funcName}(obj${accessor})
10611067
`
10621068
break
10631069
case 'array':
1064-
funcName = '$arr' + (name + key + subKey).replace(/[-.\[\] ]/g, '').replace(/[@]/g, 'AT_SYMBOL') // eslint-disable-line
1070+
funcName = asFuncName('$arr' + name + key + subKey) // eslint-disable-line
10651071
laterCode = buildArray(schema, laterCode, funcName, externalSchema, fullSchema)
10661072
code += `
10671073
json += ${funcName}(obj${accessor})
@@ -1071,7 +1077,7 @@ function nested (laterCode, name, key, schema, externalSchema, fullSchema, subKe
10711077
if ('anyOf' in schema) {
10721078
dereferenceOfRefs(schema, externalSchema, fullSchema, 'anyOf')
10731079
schema.anyOf.forEach((s, index) => {
1074-
var nestedResult = nested(laterCode, name, key, s, externalSchema, fullSchema, subKey !== '' ? subKey : 'i' + index)
1080+
var nestedResult = nested(laterCode, name, key, s, externalSchema, fullSchema, subKey !== '' ? subKey : 'i' + index, isArray)
10751081
code += `
10761082
${index === 0 ? 'if' : 'else if'}(ajv.validate(${require('util').inspect(s, { depth: null })}, obj${accessor}))
10771083
${nestedResult.code}
@@ -1084,7 +1090,7 @@ function nested (laterCode, name, key, schema, externalSchema, fullSchema, subKe
10841090
} else if ('oneOf' in schema) {
10851091
dereferenceOfRefs(schema, externalSchema, fullSchema, 'oneOf')
10861092
schema.oneOf.forEach((s, index) => {
1087-
var nestedResult = nested(laterCode, name, key, s, externalSchema, fullSchema, subKey !== '' ? subKey : 'i' + index)
1093+
var nestedResult = nested(laterCode, name, key, s, externalSchema, fullSchema, subKey !== '' ? subKey : 'i' + index, isArray)
10881094
code += `
10891095
${index === 0 ? 'if' : 'else if'}(ajv.validate(${require('util').inspect(s, { depth: null })}, obj${accessor}))
10901096
${nestedResult.code}
@@ -1108,7 +1114,7 @@ function nested (laterCode, name, key, schema, externalSchema, fullSchema, subKe
11081114
const sortedTypes = nullIndex !== -1 ? [type[nullIndex]].concat(type.slice(0, nullIndex)).concat(type.slice(nullIndex + 1)) : type
11091115
sortedTypes.forEach((type, index) => {
11101116
var tempSchema = Object.assign({}, schema, { type })
1111-
var nestedResult = nested(laterCode, name, key, tempSchema, externalSchema, fullSchema, subKey)
1117+
var nestedResult = nested(laterCode, name, key, tempSchema, externalSchema, fullSchema, subKey, isArray)
11121118

11131119
if (type === 'string') {
11141120
code += `

‎test/allof.test.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,15 +44,15 @@ test('object with allOf and multiple schema on the allOf', (t) => {
4444
id: 1
4545
})
4646
} catch (e) {
47-
t.is(e.message, 'name is required!')
47+
t.is(e.message, '"name" is required!')
4848
}
4949

5050
try {
5151
stringify({
5252
name: 'string'
5353
})
5454
} catch (e) {
55-
t.is(e.message, 'id is required!')
55+
t.is(e.message, '"id" is required!')
5656
}
5757

5858
try {

‎test/ref.test.js

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -934,7 +934,7 @@ test('Bad key', t => {
934934
})
935935
t.fail('Should throw')
936936
} catch (err) {
937-
t.is(err.message, "Cannot find reference 'porjectId', did you mean 'projectId'?")
937+
t.is(err.message, 'Cannot find reference "porjectId", did you mean "projectId"?')
938938
}
939939
})
940940

@@ -959,7 +959,7 @@ test('Bad key', t => {
959959
})
960960
t.fail('Should throw')
961961
} catch (err) {
962-
t.is(err.message, "Cannot find reference 'foobar'")
962+
t.is(err.message, 'Cannot find reference "foobar"')
963963
}
964964
})
965965

@@ -989,7 +989,7 @@ test('Bad key', t => {
989989
})
990990
t.fail('Should throw')
991991
} catch (err) {
992-
t.is(err.message, "Cannot find reference 'porjectId', did you mean 'projectId'?")
992+
t.is(err.message, 'Cannot find reference "porjectId", did you mean "projectId"?')
993993
}
994994
})
995995

@@ -1019,7 +1019,7 @@ test('Bad key', t => {
10191019
})
10201020
t.fail('Should throw')
10211021
} catch (err) {
1022-
t.is(err.message, "Cannot find reference 'foobar'")
1022+
t.is(err.message, 'Cannot find reference "foobar"')
10231023
}
10241024
})
10251025

@@ -1049,7 +1049,7 @@ test('Bad key', t => {
10491049
})
10501050
t.fail('Should throw')
10511051
} catch (err) {
1052-
t.is(err.message, "Cannot find reference 'deifnitions', did you mean 'definitions'?")
1052+
t.is(err.message, 'Cannot find reference "deifnitions", did you mean "definitions"?')
10531053
}
10541054
})
10551055

@@ -1074,7 +1074,7 @@ test('Bad key', t => {
10741074
})
10751075
t.fail('Should throw')
10761076
} catch (err) {
1077-
t.is(err.message, "Cannot find reference 'deifnitions', did you mean 'definitions'?")
1077+
t.is(err.message, 'Cannot find reference "deifnitions", did you mean "definitions"?')
10781078
}
10791079
})
10801080

@@ -1104,7 +1104,7 @@ test('Bad key', t => {
11041104
})
11051105
t.fail('Should throw')
11061106
} catch (err) {
1107-
t.is(err.message, "Cannot find reference 'extrenal', did you mean 'external'?")
1107+
t.is(err.message, 'Cannot find reference "extrenal", did you mean "external"?')
11081108
}
11091109
})
11101110

0 commit comments

Comments
 (0)