Verified Commit 253f82ea authored by Aral Balkan's avatar Aral Balkan
Browse files

Closes #10: sanitise query input to avoid injection attacks

parent 625dbddf
......@@ -40,7 +40,46 @@ class QueryProxy {
get cachedResult () {
if (this.#cachedResult === null) {
this.#cachedResult = this.data.filter(valueOf => { return eval(this.query) })
//
// Sanitise the query
//
// Remove statement terminators, etc. Sorry, Little Bobby Tables.
this.query = this.query.replace(/[;\\\+\`]/g, '')
// Now let’s see if there’s anything nefarious left after we strip away
// the things we expect to be there. This isn’t perfect if the attacker
// knows enough to surround their attacks using valueOf assignments
// but it will catch arbitrary attempts, especially if the codebase
// uses whereIsTrue() instead of where() and return without traversing the
// data graph. Anything not caught here will trigger an error during
// the array filter
let sieve = this.query
.replace(/valueOf\..+?\.toLowerCase()\.(startsWith|endsWith|includes|startsWithCaseInsensitive|endsWithCaseInsensitive|includesCaseInsensitive)\(.+?\)/g, '')
.replace(/valueOf\..+?\.(startsWith|endsWith|includes|startsWithCaseInsensitive|endsWithCaseInsensitive|includesCaseInsensitive)\(.+?\)/g, '')
.replace(/valueOf\..+?\s?(===|!==|<|>|<=|>=)\s?([\d_\.]+\s?|'.+?'|".+?")/g, '')
.replace(/\|\|/g, '')
.replace(/\&\&/g,'')
.replace(/['"\(\)\s]/g, '')
if (sieve !== '') {
// There is still something in our sieve and there shouldn’t be.
// Reject this as a possible arbitrary code execustion attack via injection.
this.#cachedResult = []
} else {
// OK, this query should be safe to run. Note that it is still possible
// that there is a malformed injection attack and this will cause an error
// on eval. We catch and handle that accordingly, below.
this.#cachedResult = this.data.filter(valueOf => {
try {
return eval(this.query)
} catch (error) {
// Possible injection attack, return false.
return false
}
})
}
}
return this.#cachedResult
}
......
......@@ -647,6 +647,35 @@ test('Basic queries', t => {
// This should have restored the array to its previous state, after the push.
t.strictEquals(JSON.stringify(completedQuery.get()), JSON.stringify(expectedResultsetAfterPush), 'defining a property on the resultset array works as expected')
//
// Query security.
// See: https://source.small-tech.org/site.js/lib/jsdb/-/issues/10
//
// Temporarily create a global reference to the current test so that the
// attack payloads can use it to fail the tests should they succeed.
globalThis.t = t
const queryInjectionAttackAttemptResult1 = db.cars.where('make === "something"; globalThis.t.fail("Query injection payload 1 delivered"); valueOf.make').is('something'
).get()
const queryInjectionAttackAttemptResult2 = db.cars.where('make').is('\'+globalThis.t.fail("Query injection payload 2 delivered")+\'').get()
const queryInjectionAttackAttemptResult3 = db.cars.whereIsTrue('globalThis.t.fail("Query injection payload 3 delivered")').get()
const queryInjectionAttackAttemptResult4 = db.cars.whereIsTrue('valueOf.make === "something"; globalThis.t.fail("Query injection payload 4 delivered"); valueOf.make === \'something\'').get()
const queryInjectionAttackAttemptResult5 = db.cars.whereIsTrue('valueOf.make === `something`; globalThis.t.fail("Query injection payload 5 delivered"); valueOf.make === \'something\'').get()
// Remove the global reference to the test instance as it’s no longer necessary.
globalThis.t = null
t.ok(Array.isArray(queryInjectionAttackAttemptResult1) && queryInjectionAttackAttemptResult1.length === 0, '🔒 result of query injection attack attempt 1 is empty array as expected')
t.ok(Array.isArray(queryInjectionAttackAttemptResult2) && queryInjectionAttackAttemptResult2.length === 0, '🔒 result of query injection attack attempt 2 is empty array as expected')
t.ok(Array.isArray(queryInjectionAttackAttemptResult3) && queryInjectionAttackAttemptResult3.length === 0, '🔒 result of query injection attack attempt 3 is empty array as expected')
t.ok(Array.isArray(queryInjectionAttackAttemptResult4) && queryInjectionAttackAttemptResult4.length === 0, '🔒 result of query injection attack attempt 4 is empty array as expected')
t.ok(Array.isArray(queryInjectionAttackAttemptResult5) && queryInjectionAttackAttemptResult5.length === 0, '🔒 result of query injection attack attempt 5 is empty array as expected')
t.end()
})
......@@ -803,9 +832,9 @@ test('JSDF', t => {
// Security
testSerialisation(t, 'String injection attempt 1 fails as expected', "${t.fail('Payload 1 delivered')}")
testSerialisation(t, 'String injection attempt 2 fails as expected', "\\${t.fail('Payload 2 delivered')}")
testSerialisation(t, 'String injection attempt 3 fails as expected', "` + t.fail('Payload 3 delivered') + `")
testSerialisation(t, '🔒 String injection attempt 1 fails as expected', "${t.fail('Payload 1 delivered')}")
testSerialisation(t, '🔒 String injection attempt 2 fails as expected', "\\${t.fail('Payload 2 delivered')}")
testSerialisation(t, '🔒 String injection attempt 3 fails as expected', "` + t.fail('Payload 3 delivered') + `")
//
// Plain objects.
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment