Verified Commit b7f34f7e authored by Aral Balkan's avatar Aral Balkan
Browse files

Merge branch 'sanitise-queries'

parents 625dbddf 6afe6a9c
...@@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file. ...@@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
## [1.0.0] - 2019-10-19 ## [1.1.0] - 2020-10-21
### Added
- Now sanitising queries to prevent arbitrary code execution via injection attacks.
## [1.0.0] - 2020-10-19
Initial release. Initial release.
...@@ -139,8 +139,17 @@ Just because it’s JavaScript, it doesn’t mean that you can throw anything in ...@@ -139,8 +139,17 @@ Just because it’s JavaScript, it doesn’t mean that you can throw anything in
Additionally, `null` and `undefined` values will be persisted as-is. Additionally, `null` and `undefined` values will be persisted as-is.
### Security note regarding strings
Strings are automatically sanitised to escape backticks, backslashes, and template placeholder tokens to avoid arbitrary code execution via JavaScript injection attacks. Strings are automatically sanitised to escape backticks, backslashes, and template placeholder tokens to avoid arbitrary code execution via JavaScript injection attacks.
The relevant areas in the codebase are linked to below.
- [String sanitation code (JSDF class)](https://github.com/small-tech/jsdb/blob/master/lib/JSDF.js#L45)
- [String sanitation code tests (test/index.js)](https://github.com/small-tech/jsdb/blob/master/test/index.js#L804)
If you notice anything we’ve overlooked or if you have suggestions for improvements, [please open an issue](https://github.com/small-tech/jsdb/issues).
### Custom data types ### Custom data types
Custom data types (instances of your own classes) are also supported. Custom data types (instances of your own classes) are also supported.
...@@ -685,6 +694,33 @@ const carsThatAreRegal = db.cars.where('tags').includes('regal').get() ...@@ -685,6 +694,33 @@ const carsThatAreRegal = db.cars.where('tags').includes('regal').get()
] ]
``` ```
### Security considerations with queries
JSDB (as of version 1.1.0), attempts to sanitise your queries for you to avoid [Little Bobby Tables](https://xkcd.com/327/).
The current sanitation strategy is two-fold and is executed at time of query execution:
1. Remove dangerous characters (statement terminators, etc.):
- Semi-colon (`;`)
- Backslash (`\`)
- Backtick (`\``)
- Plus sign (`+`)
- Dollar sign (`$`)
- Curly brackets (`{}`)
Reasoning: remove symbols that could be used to create valid code so that if our sieve (see below) doesn’t catch an attempt, the code will throw an error when executed, which we can catch and handle.
2. Use a sieve to remove expected input. If our sieve contains any leftover material, we immediately return an empty result set without executing the query.
During query execution, if the query throws (due to an injection attempt that as been neutralised at Step 1 but made it through the sieve), we simply catch the error and return an empty result set.
The relevant areas in the codebase are linked to below.
- [Query sanitation code (QueryProxy class)](https://github.com/small-tech/jsdb/blob/master/lib/QueryProxy.js#L44)
- [Query sanitation code tests (test/index.js)](https://github.com/small-tech/jsdb/blob/master/test/index.js#L650)
If you notice anything we’ve overlooked or if you have suggestions for improvements, [please open an issue](https://github.com/small-tech/jsdb/issues).
## Performance characteristics ## Performance characteristics
- The time complexity of reads and writes are both O(1). - The time complexity of reads and writes are both O(1).
......
...@@ -40,7 +40,46 @@ class QueryProxy { ...@@ -40,7 +40,46 @@ class QueryProxy {
get cachedResult () { get cachedResult () {
if (this.#cachedResult === null) { 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 return this.#cachedResult
} }
......
{ {
"name": "@small-tech/jsdb", "name": "@small-tech/jsdb",
"version": "1.0.0", "version": "1.1.0",
"description": "A transparent, in-memory, streaming write-on-update JavaScript database for Small Web applications that persists to a JavaScript transaction log.", "description": "A transparent, in-memory, streaming write-on-update JavaScript database for Small Web applications that persists to a JavaScript transaction log.",
"keywords": [ "keywords": [
"js", "js",
......
...@@ -647,6 +647,35 @@ test('Basic queries', t => { ...@@ -647,6 +647,35 @@ test('Basic queries', t => {
// This should have restored the array to its previous state, after the push. // 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') 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() t.end()
}) })
...@@ -803,9 +832,9 @@ test('JSDF', t => { ...@@ -803,9 +832,9 @@ test('JSDF', t => {
// Security // Security
testSerialisation(t, 'String injection attempt 1 fails as expected', "${t.fail('Payload 1 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 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 3 fails as expected', "` + t.fail('Payload 3 delivered') + `")
// //
// Plain objects. // 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