From d64c977c37171b7b525b1e996a2851a664023417 Mon Sep 17 00:00:00 2001 From: Andrey Sidorov Date: Sun, 21 Apr 2024 20:31:05 +1000 Subject: [PATCH 1/2] fix(security): sanitize timezone parameter value to prevent code injection --- lib/parsers/binary_parser.js | 4 ++-- lib/parsers/text_parser.js | 4 ++-- .../timezone-binary-sanitization.test.mjs | 24 +++++++++++++++++++ .../timezone-text-sanitization.test.mjs | 24 +++++++++++++++++++ 4 files changed, 52 insertions(+), 4 deletions(-) create mode 100644 test/esm/unit/parsers/timezone-binary-sanitization.test.mjs create mode 100644 test/esm/unit/parsers/timezone-text-sanitization.test.mjs diff --git a/lib/parsers/binary_parser.js b/lib/parsers/binary_parser.js index 1083204f04..9d53d8c188 100644 --- a/lib/parsers/binary_parser.js +++ b/lib/parsers/binary_parser.js @@ -42,9 +42,9 @@ function readCodeFor(field, config, options, fieldNum) { case Types.TIMESTAMP: case Types.NEWDATE: if (helpers.typeMatch(field.columnType, dateStrings, Types)) { - return `packet.readDateTimeString(${field.decimals});`; + return `packet.readDateTimeString(${parseInt(field.decimals, 10)});`; } - return `packet.readDateTime('${timezone}');`; + return `packet.readDateTime(${helpers.srcEscape(timezone)});`; case Types.TIME: return 'packet.readTimeString()'; case Types.DECIMAL: diff --git a/lib/parsers/text_parser.js b/lib/parsers/text_parser.js index 7e46514463..be89e77123 100644 --- a/lib/parsers/text_parser.js +++ b/lib/parsers/text_parser.js @@ -48,13 +48,13 @@ function readCodeFor(type, charset, encodingExpr, config, options) { if (helpers.typeMatch(type, dateStrings, Types)) { return 'packet.readLengthCodedString("ascii")'; } - return `packet.parseDate('${timezone}')`; + return `packet.parseDate(${helpers.srcEscape(timezone)})`; case Types.DATETIME: case Types.TIMESTAMP: if (helpers.typeMatch(type, dateStrings, Types)) { return 'packet.readLengthCodedString("ascii")'; } - return `packet.parseDateTime('${timezone}')`; + return `packet.parseDateTime(${helpers.srcEscape(timezone)})`; case Types.TIME: return 'packet.readLengthCodedString("ascii")'; case Types.GEOMETRY: diff --git a/test/esm/unit/parsers/timezone-binary-sanitization.test.mjs b/test/esm/unit/parsers/timezone-binary-sanitization.test.mjs new file mode 100644 index 0000000000..e0943e6344 --- /dev/null +++ b/test/esm/unit/parsers/timezone-binary-sanitization.test.mjs @@ -0,0 +1,24 @@ +import { describe, test, assert } from 'poku'; +import { createConnection, describeOptions } from '../../../common.test.cjs'; + +const connection = createConnection().promise(); + +describe('Binary Parser: timezone Sanitization', describeOptions); + +Promise.all([ + test(async () => { + process.env.TEST_ENV_VALUE = "secure"; + await connection.execute({ + sql: 'SELECT NOW()', + timezone: `'); process.env.TEST_ENV_VALUE = "not so much"; //`, + }); + + assert.strictEqual( + process.env.TEST_ENV_VALUE, + 'secure', + 'Timezone sanitization failed - code injection possible', + ); + }), +]).then(async () => { + await connection.end(); +}); diff --git a/test/esm/unit/parsers/timezone-text-sanitization.test.mjs b/test/esm/unit/parsers/timezone-text-sanitization.test.mjs new file mode 100644 index 0000000000..8159d49275 --- /dev/null +++ b/test/esm/unit/parsers/timezone-text-sanitization.test.mjs @@ -0,0 +1,24 @@ +import { describe, test, assert } from 'poku'; +import { createConnection, describeOptions } from '../../../common.test.cjs'; + +const connection = createConnection().promise(); + +describe('Text Parser: timezone Sanitization', describeOptions); + +Promise.all([ + test(async () => { + process.env.TEST_ENV_VALUE = "secure"; + await connection.query({ + sql: 'SELECT NOW()', + timezone: `'); process.env.TEST_ENV_VALUE = "not so much"; //`, + }); + + assert.strictEqual( + process.env.TEST_ENV_VALUE, + 'secure', + 'Timezone sanitization failed - code injection possible', + ); + }), +]).then(async () => { + await connection.end(); +}); From 21f63446174f221d081681b6f2d050af6510eb7f Mon Sep 17 00:00:00 2001 From: Andrey Sidorov Date: Sun, 21 Apr 2024 20:36:10 +1000 Subject: [PATCH 2/2] lint --- test/esm/unit/parsers/timezone-binary-sanitization.test.mjs | 2 +- test/esm/unit/parsers/timezone-text-sanitization.test.mjs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/esm/unit/parsers/timezone-binary-sanitization.test.mjs b/test/esm/unit/parsers/timezone-binary-sanitization.test.mjs index e0943e6344..03b2591af9 100644 --- a/test/esm/unit/parsers/timezone-binary-sanitization.test.mjs +++ b/test/esm/unit/parsers/timezone-binary-sanitization.test.mjs @@ -7,7 +7,7 @@ describe('Binary Parser: timezone Sanitization', describeOptions); Promise.all([ test(async () => { - process.env.TEST_ENV_VALUE = "secure"; + process.env.TEST_ENV_VALUE = 'secure'; await connection.execute({ sql: 'SELECT NOW()', timezone: `'); process.env.TEST_ENV_VALUE = "not so much"; //`, diff --git a/test/esm/unit/parsers/timezone-text-sanitization.test.mjs b/test/esm/unit/parsers/timezone-text-sanitization.test.mjs index 8159d49275..82ba9546a9 100644 --- a/test/esm/unit/parsers/timezone-text-sanitization.test.mjs +++ b/test/esm/unit/parsers/timezone-text-sanitization.test.mjs @@ -7,7 +7,7 @@ describe('Text Parser: timezone Sanitization', describeOptions); Promise.all([ test(async () => { - process.env.TEST_ENV_VALUE = "secure"; + process.env.TEST_ENV_VALUE = 'secure'; await connection.query({ sql: 'SELECT NOW()', timezone: `'); process.env.TEST_ENV_VALUE = "not so much"; //`,