From 74abf9ef94d76114d9a09415e28b496522a94805 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Weslley=20Ara=C3=BAjo?= <46850407+wellwelwel@users.noreply.github.com> Date: Mon, 8 Apr 2024 22:38:11 -0300 Subject: [PATCH] fix(security): improve supportBigNumbers and bigNumberStrings sanitization (#2572) Fixes a potential RCE attack vulnerability reported by Vsevolod Kokorin (Slonser) of Solidlab --- lib/parsers/binary_parser.js | 9 ++- lib/parsers/text_parser.js | 47 +++++++------ .../big-numbers-strings-sanitization.test.mjs | 66 +++++++++++++++++++ .../support-big-numbers-sanitization.test.mjs | 62 +++++++++++++++++ 4 files changed, 160 insertions(+), 24 deletions(-) create mode 100644 test/esm/unit/parsers/big-numbers-strings-sanitization.test.mjs create mode 100644 test/esm/unit/parsers/support-big-numbers-sanitization.test.mjs diff --git a/lib/parsers/binary_parser.js b/lib/parsers/binary_parser.js index 431606891e..b9ee92ffe6 100644 --- a/lib/parsers/binary_parser.js +++ b/lib/parsers/binary_parser.js @@ -12,9 +12,12 @@ for (const t in Types) { } function readCodeFor(field, config, options, fieldNum) { - const supportBigNumbers = - options.supportBigNumbers || config.supportBigNumbers; - const bigNumberStrings = options.bigNumberStrings || config.bigNumberStrings; + const supportBigNumbers = Boolean( + options.supportBigNumbers || config.supportBigNumbers, + ); + const bigNumberStrings = Boolean( + options.bigNumberStrings || config.bigNumberStrings, + ); const timezone = options.timezone || config.timezone; const dateStrings = options.dateStrings || config.dateStrings; const unsigned = field.flags & FieldFlags.UNSIGNED; diff --git a/lib/parsers/text_parser.js b/lib/parsers/text_parser.js index 49a128c049..23d0d38629 100644 --- a/lib/parsers/text_parser.js +++ b/lib/parsers/text_parser.js @@ -12,9 +12,12 @@ for (const t in Types) { } function readCodeFor(type, charset, encodingExpr, config, options) { - const supportBigNumbers = - options.supportBigNumbers || config.supportBigNumbers; - const bigNumberStrings = options.bigNumberStrings || config.bigNumberStrings; + const supportBigNumbers = Boolean( + options.supportBigNumbers || config.supportBigNumbers, + ); + const bigNumberStrings = Boolean( + options.bigNumberStrings || config.bigNumberStrings, + ); const timezone = options.timezone || config.timezone; const dateStrings = options.dateStrings || config.dateStrings; @@ -85,22 +88,24 @@ function compile(fields, options, config) { db: field.schema, table: field.table, name: field.name, - string: function(encoding = field.encoding) { + string: function (encoding = field.encoding) { if (field.columnType === Types.JSON && encoding === field.encoding) { // Since for JSON columns mysql always returns charset 63 (BINARY), // we have to handle it according to JSON specs and use "utf8", // see https://github.com/sidorares/node-mysql2/issues/1661 - console.warn(`typeCast: JSON column "${field.name}" is interpreted as BINARY by default, recommended to manually set utf8 encoding: \`field.string("utf8")\``); + console.warn( + `typeCast: JSON column "${field.name}" is interpreted as BINARY by default, recommended to manually set utf8 encoding: \`field.string("utf8")\``, + ); } return _this.packet.readLengthCodedString(encoding); }, - buffer: function() { + buffer: function () { return _this.packet.readLengthCodedBuffer(); }, - geometry: function() { + geometry: function () { return _this.packet.parseGeometryValue(); - } + }, }; } @@ -109,9 +114,7 @@ function compile(fields, options, config) { /* eslint-disable no-trailing-spaces */ /* eslint-disable no-spaced-func */ /* eslint-disable no-unexpected-multiline */ - parserFn('(function () {')( - 'return class TextRow {' - ); + parserFn('(function () {')('return class TextRow {'); // constructor method parserFn('constructor(fields) {'); @@ -127,22 +130,22 @@ function compile(fields, options, config) { // next method parserFn('next(packet, fields, options) {'); - parserFn("this.packet = packet;"); + parserFn('this.packet = packet;'); if (options.rowsAsArray) { parserFn(`const result = new Array(${fields.length});`); } else { - parserFn("const result = {};"); + parserFn('const result = {};'); } const resultTables = {}; let resultTablesArray = []; if (options.nestTables === true) { - for (let i=0; i < fields.length; i++) { + for (let i = 0; i < fields.length; i++) { resultTables[fields[i].table] = 1; } resultTablesArray = Object.keys(resultTables); - for (let i=0; i < resultTablesArray.length; i++) { + for (let i = 0; i < resultTablesArray.length; i++) { parserFn(`result[${helpers.srcEscape(resultTablesArray[i])}] = {};`); } } @@ -154,7 +157,7 @@ function compile(fields, options, config) { parserFn(`// ${fieldName}: ${typeNames[fields[i].columnType]}`); if (typeof options.nestTables === 'string') { lvalue = `result[${helpers.srcEscape( - fields[i].table + options.nestTables + fields[i].name + fields[i].table + options.nestTables + fields[i].name, )}]`; } else if (options.nestTables === true) { lvalue = `result[${helpers.srcEscape(fields[i].table)}][${fieldName}]`; @@ -172,11 +175,13 @@ function compile(fields, options, config) { fields[i].characterSet, encodingExpr, config, - options + options, ); if (typeof options.typeCast === 'function') { - parserFn(`${lvalue} = options.typeCast(this.wrap${i}, function() { return ${readCode} });`); - } else { + parserFn( + `${lvalue} = options.typeCast(this.wrap${i}, function() { return ${readCode} });`, + ); + } else { parserFn(`${lvalue} = ${readCode};`); } } @@ -193,11 +198,11 @@ function compile(fields, options, config) { if (config.debug) { helpers.printDebugWithCode( 'Compiled text protocol row parser', - parserFn.toString() + parserFn.toString(), ); } if (typeof options.typeCast === 'function') { - return parserFn.toFunction({wrap}); + return parserFn.toFunction({ wrap }); } return parserFn.toFunction(); } diff --git a/test/esm/unit/parsers/big-numbers-strings-sanitization.test.mjs b/test/esm/unit/parsers/big-numbers-strings-sanitization.test.mjs new file mode 100644 index 0000000000..09cc6a2d81 --- /dev/null +++ b/test/esm/unit/parsers/big-numbers-strings-sanitization.test.mjs @@ -0,0 +1,66 @@ +import { describe, test, assert } from 'poku'; +import { createConnection, describeOptions } from '../../../common.test.cjs'; + +const connection = createConnection().promise(); + +const sql = 'SELECT 9007199254740991+100 AS `total`'; + +describe('bigNumberStrings Sanitization', describeOptions); + +Promise.all([ + test(async () => { + const [results] = await connection.query({ + sql, + supportBigNumbers: true, + bigNumberStrings: true, + }); + + assert.strictEqual( + typeof results[0].total, + 'string', + 'Valid bigNumberStrings enabled', + ); + }), + test(async () => { + const [results] = await connection.query({ + sql, + supportBigNumbers: false, + bigNumberStrings: false, + }); + + assert.strictEqual( + typeof results[0].total, + 'number', + 'Valid bigNumberStrings disabled', + ); + }), + + test(async () => { + const [results] = await connection.query({ + sql, + supportBigNumbers: 'text', + bigNumberStrings: 'text', + }); + + assert.strictEqual( + typeof results[0].total, + 'string', + 'bigNumberStrings as a random string should be enabled', + ); + }), + test(async () => { + const [results] = await connection.query({ + sql, + supportBigNumbers: '', + bigNumberStrings: '', + }); + + assert.strictEqual( + typeof results[0].total, + 'number', + 'bigNumberStrings as an empty string should be disabled', + ); + }), +]).then(async () => { + await connection.end(); +}); diff --git a/test/esm/unit/parsers/support-big-numbers-sanitization.test.mjs b/test/esm/unit/parsers/support-big-numbers-sanitization.test.mjs new file mode 100644 index 0000000000..174ed76ffa --- /dev/null +++ b/test/esm/unit/parsers/support-big-numbers-sanitization.test.mjs @@ -0,0 +1,62 @@ +import { describe, test, assert } from 'poku'; +import { createConnection, describeOptions } from '../../../common.test.cjs'; + +const connection = createConnection().promise(); + +const sql = 'SELECT 9007199254740991+100 AS `total`'; + +describe('supportBigNumbers Sanitization', describeOptions); + +Promise.all([ + test(async () => { + const [results] = await connection.query({ + sql, + supportBigNumbers: true, + }); + + assert.strictEqual( + typeof results[0].total, + 'string', + 'Valid supportBigNumbers enabled', + ); + }), + test(async () => { + const [results] = await connection.query({ + sql, + supportBigNumbers: false, + }); + + assert.strictEqual( + typeof results[0].total, + 'number', + 'Valid supportBigNumbers disabled', + ); + }), + + test(async () => { + const [results] = await connection.query({ + sql, + supportBigNumbers: 'text', + }); + + assert.strictEqual( + typeof results[0].total, + 'string', + 'supportBigNumbers as a random string should be enabled', + ); + }), + test(async () => { + const [results] = await connection.query({ + sql, + supportBigNumbers: '', + }); + + assert.strictEqual( + typeof results[0].total, + 'number', + 'supportBigNumbers as an empty string should be disabled', + ); + }), +]).then(async () => { + await connection.end(); +});