Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable prototype poisoning option #1414

Merged
merged 5 commits into from
Mar 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/basic-config.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -244,4 +244,8 @@ const client = new Client({
})
----

|`disablePrototypePoisoningProtection`
|`boolean`, `'proto'`, `'constructor'` - By the default the client will protect you against prototype poisoning attacks. Read https://web.archive.org/web/20200319091159/https://hueniverse.com/square-brackets-are-the-enemy-ff5b9fd8a3e8?gi=184a27ee2a08[this article] to learn more. If needed you can disable prototype poisoning protection entirely or one of the two checks. Read the `secure-json-parse` https://github.com/fastify/secure-json-parse[documentation] to learn more. +
_Default:_ `false`

|===
3 changes: 2 additions & 1 deletion index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,8 @@ interface ClientOptions {
// TODO: remove username and password here in 8
username?: string;
password?: string;
}
};
disablePrototypePoisoningProtection?: boolean | 'proto' | 'constructor';
}

declare class Client {
Expand Down
7 changes: 5 additions & 2 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,8 @@ class Client extends ESAPI {
opaqueIdPrefix: null,
context: null,
proxy: null,
enableMetaHeader: true
enableMetaHeader: true,
disablePrototypePoisoningProtection: false
}, opts)

this[kInitialOptions] = options
Expand All @@ -140,7 +141,9 @@ class Client extends ESAPI {
this[kEventEmitter] = options[kChild].eventEmitter
} else {
this[kEventEmitter] = new EventEmitter()
this.serializer = new options.Serializer()
this.serializer = new options.Serializer({
disablePrototypePoisoningProtection: options.disablePrototypePoisoningProtection
})
this.connectionPool = new options.ConnectionPool({
pingTimeout: options.pingTimeout,
resurrectStrategy: options.resurrectStrategy,
Expand Down
22 changes: 11 additions & 11 deletions lib/Helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@ class Helpers {
*/
bulk (options, reqOptions = {}) {
const client = this[kClient]
const { serialize, deserialize } = client.serializer
const { serializer } = client
if (this[kMetaHeader] !== null) {
reqOptions.headers = reqOptions.headers || {}
reqOptions.headers['x-elastic-client-meta'] = this[kMetaHeader] + ',h=bp'
Expand Down Expand Up @@ -505,19 +505,19 @@ class Helpers {
? Object.keys(action[0])[0]
: Object.keys(action)[0]
if (operation === 'index' || operation === 'create') {
actionBody = serialize(action)
payloadBody = typeof chunk === 'string' ? chunk : serialize(chunk)
actionBody = serializer.serialize(action)
payloadBody = typeof chunk === 'string' ? chunk : serializer.serialize(chunk)
chunkBytes += Buffer.byteLength(actionBody) + Buffer.byteLength(payloadBody)
bulkBody.push(actionBody, payloadBody)
} else if (operation === 'update') {
actionBody = serialize(action[0])
actionBody = serializer.serialize(action[0])
payloadBody = typeof chunk === 'string'
? `{"doc":${chunk}}`
: serialize({ doc: chunk, ...action[1] })
: serializer.serialize({ doc: chunk, ...action[1] })
chunkBytes += Buffer.byteLength(actionBody) + Buffer.byteLength(payloadBody)
bulkBody.push(actionBody, payloadBody)
} else if (operation === 'delete') {
actionBody = serialize(action)
actionBody = serializer.serialize(action)
chunkBytes += Buffer.byteLength(actionBody)
bulkBody.push(actionBody)
} else {
Expand Down Expand Up @@ -669,13 +669,13 @@ class Helpers {
return
}
for (let i = 0, len = bulkBody.length; i < len; i = i + 2) {
const operation = Object.keys(deserialize(bulkBody[i]))[0]
const operation = Object.keys(serializer.deserialize(bulkBody[i]))[0]
onDrop({
status: 429,
error: null,
operation: deserialize(bulkBody[i]),
operation: serializer.deserialize(bulkBody[i]),
document: operation !== 'delete'
? deserialize(bulkBody[i + 1])
? serializer.deserialize(bulkBody[i + 1])
/* istanbul ignore next */
: null,
retried: isRetrying
Expand Down Expand Up @@ -716,9 +716,9 @@ class Helpers {
onDrop({
status: status,
error: action[operation].error,
operation: deserialize(bulkBody[indexSlice]),
operation: serializer.deserialize(bulkBody[indexSlice]),
document: operation !== 'delete'
? deserialize(bulkBody[indexSlice + 1])
? serializer.deserialize(bulkBody[indexSlice + 1])
: null,
retried: isRetrying
})
Expand Down
5 changes: 5 additions & 0 deletions lib/Serializer.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,12 @@
* under the License.
*/

export interface SerializerOptions {
disablePrototypePoisoningProtection: boolean | 'proto' | 'constructor'
}

export default class Serializer {
constructor (opts?: SerializerOptions)
serialize(object: any): string;
deserialize(json: string): any;
ndserialize(array: any[]): string;
Expand Down
11 changes: 10 additions & 1 deletion lib/Serializer.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,17 @@ const { stringify } = require('querystring')
const debug = require('debug')('elasticsearch')
const sjson = require('secure-json-parse')
const { SerializationError, DeserializationError } = require('./errors')
const kJsonOptions = Symbol('secure json parse options')

class Serializer {
constructor (opts = {}) {
const disable = opts.disablePrototypePoisoningProtection
this[kJsonOptions] = {
protoAction: disable === true || disable === 'proto' ? 'ignore' : 'error',
constructorAction: disable === true || disable === 'constructor' ? 'ignore' : 'error'
}
}

serialize (object) {
debug('Serializing', object)
let json
Expand All @@ -40,7 +49,7 @@ class Serializer {
debug('Deserializing', json)
let object
try {
object = sjson.parse(json)
object = sjson.parse(json, this[kJsonOptions])
} catch (err) {
throw new DeserializationError(err.message, json)
}
Expand Down
16 changes: 0 additions & 16 deletions test/types/client-options.test-d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -557,22 +557,6 @@ expectError<errors.ConfigurationError>(
)
}

{
class CustomSerializer {
deserialize (str: string) {
return JSON.parse(str)
}
}

expectError<errors.ConfigurationError>(
// @ts-expect-error
new Client({
node: 'http://localhost:9200',
Serializer: CustomSerializer
})
)
}

/**
* `Connection` option
*/
Expand Down
57 changes: 57 additions & 0 deletions test/unit/client.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1364,3 +1364,60 @@ test('Meta header disabled', t => {
t.error(err)
})
})

test('Prototype poisoning protection enabled by default', t => {
t.plan(1)

class MockConnection extends Connection {
request (params, callback) {
const stream = intoStream('{"__proto__":{"foo":"bar"}}')
stream.statusCode = 200
stream.headers = {
'content-type': 'application/json;utf=8',
'content-length': '27',
connection: 'keep-alive',
date: new Date().toISOString()
}
process.nextTick(callback, null, stream)
return { abort () {} }
}
}

const client = new Client({
node: 'http://localhost:9200',
Connection: MockConnection
})

client.info((err, result) => {
t.true(err instanceof errors.DeserializationError)
})
})

test('Disable prototype poisoning protection', t => {
t.plan(1)

class MockConnection extends Connection {
request (params, callback) {
const stream = intoStream('{"__proto__":{"foo":"bar"}}')
stream.statusCode = 200
stream.headers = {
'content-type': 'application/json;utf=8',
'content-length': '27',
connection: 'keep-alive',
date: new Date().toISOString()
}
process.nextTick(callback, null, stream)
return { abort () {} }
}
}

const client = new Client({
node: 'http://localhost:9200',
Connection: MockConnection,
disablePrototypePoisoningProtection: true
})

client.info((err, result) => {
t.error(err)
})
})
72 changes: 72 additions & 0 deletions test/unit/serializer.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,3 +157,75 @@ test('DeserializationError', t => {
t.ok(err instanceof DeserializationError)
}
})

test('prototype poisoning protection', t => {
t.plan(2)
const s = new Serializer()
try {
s.deserialize('{"__proto__":{"foo":"bar"}}')
t.fail('Should fail')
} catch (err) {
t.ok(err instanceof DeserializationError)
}

try {
s.deserialize('{"constructor":{"prototype":{"foo":"bar"}}}')
t.fail('Should fail')
} catch (err) {
t.ok(err instanceof DeserializationError)
}
})

test('disable prototype poisoning protection', t => {
t.plan(2)
const s = new Serializer({ disablePrototypePoisoningProtection: true })
try {
s.deserialize('{"__proto__":{"foo":"bar"}}')
t.pass('Should not fail')
} catch (err) {
t.fail(err)
}

try {
s.deserialize('{"constructor":{"prototype":{"foo":"bar"}}}')
t.pass('Should not fail')
} catch (err) {
t.fail(err)
}
})

test('disable prototype poisoning protection only for proto', t => {
t.plan(2)
const s = new Serializer({ disablePrototypePoisoningProtection: 'proto' })
try {
s.deserialize('{"__proto__":{"foo":"bar"}}')
t.pass('Should not fail')
} catch (err) {
t.fail(err)
}

try {
s.deserialize('{"constructor":{"prototype":{"foo":"bar"}}}')
t.fail('Should fail')
} catch (err) {
t.ok(err instanceof DeserializationError)
}
})

test('disable prototype poisoning protection only for constructor', t => {
t.plan(2)
const s = new Serializer({ disablePrototypePoisoningProtection: 'constructor' })
try {
s.deserialize('{"__proto__":{"foo":"bar"}}')
t.fail('Should fail')
} catch (err) {
t.ok(err instanceof DeserializationError)
}

try {
s.deserialize('{"constructor":{"prototype":{"foo":"bar"}}}')
t.pass('Should not fail')
} catch (err) {
t.fail(err)
}
})