CVE-2024-21509: Insufficient Information (NVD-CWE-noinfo)
value: Versions of the package mysql2 before 3.9.4 are vulnerable to Prototype Poisoning due to insecure results object creation and improper user input sanitization passed through parserFn in text_parser.js and binary_parser.js.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
Published: 2024-04-10T05:15Z
Repository: https://github.com/sidorares/node-mysql2
Language: JavaScript
Files changed: .nycrc, lib/parsers/binary_parser.js, lib/parsers/text_parser.js, test/common.test.cjs, test/esm/unit/parsers/prototype-binary-results.test.mjs, test/esm/unit/parsers/prototype-text-results.test.mjs
Commit date: 2024-04-09 04:02:39 -0300
Commit message: fix(security): improve results object creation (#2574)

Fixes a potential Prototype Pollution attack vulnerability reported by Vsevolod Kokorin (Slonser) of Solidlab
Diff: diff --git a/.nycrc b/.nycrc
index 04e485a2..a0d7e753 100644
--- a/.nycrc
+++ b/.nycrc
@@ -3,10 +3,10 @@
   "include": ["index.js", "promise.js", "lib/**/*.js"],
   "exclude": ["mysqldata/**", "node_modules/**", "test/**"],
   "reporter": ["text", "lcov", "cobertura"],
   "statements": 88,
   "branches": 84,
-  "functions": 78,
+  "functions": 77,
   "lines": 88,
   "checkCoverage": true,
   "clean": true
 }
diff --git a/lib/parsers/binary_parser.js b/lib/parsers/binary_parser.js
index b9ee92ff..26ac7893 100644
--- a/lib/parsers/binary_parser.js
+++ b/lib/parsers/binary_parser.js
@@ -120,11 +120,17 @@ function compile(fields, options, config) {
 
   parserFn('next(packet, fields, options) {');
   if (options.rowsAsArray) {
     parserFn(`const result = new Array(${fields.length});`);
   } else {
-    parserFn('const result = {};');
+    parserFn('const result = Object.create(null);');
+    parserFn(`Object.defineProperty(result, "constructor", {
+      value: Object.create(null),
+      writable: false,
+      configurable: false,
+      enumerable: false
+    });`);
   }
 
   // Global typeCast
   if (
     typeof config.typeCast === 'function' &&
@@ -152,11 +158,13 @@ function compile(fields, options, config) {
       lvalue = `result[${helpers.srcEscape(
         fields[i].table + options.nestTables + fields[i].name,
       )}]`;
     } else if (options.nestTables === true) {
       tableName = helpers.srcEscape(fields[i].table);
-      parserFn(`if (!result[${tableName}]) result[${tableName}] = {};`);
+      parserFn(
+        `if (!result[${tableName}]) result[${tableName}] = Object.create(null);`,
+      );
       lvalue = `result[${tableName}][${fieldName}]`;
     } else if (options.rowsAsArray) {
       lvalue = `result[${i.toString(10)}]`;
     } else {
       lvalue = `result[${fieldName}]`;
diff --git a/lib/parsers/text_parser.js b/lib/parsers/text_parser.js
index 23d0d386..e757b456 100644
--- a/lib/parsers/text_parser.js
+++ b/lib/parsers/text_parser.js
@@ -109,13 +109,10 @@ function compile(fields, options, config) {
     };
   }
 
   const parserFn = genFunc();
 
-  /* eslint-disable no-trailing-spaces */
-  /* eslint-disable no-spaced-func */
-  /* eslint-disable no-unexpected-multiline */
   parserFn('(function () {')('return class TextRow {');
 
   // constructor method
   parserFn('constructor(fields) {');
   // node-mysql typeCast compatibility wrapper
@@ -132,11 +129,17 @@ function compile(fields, options, config) {
   parserFn('next(packet, fields, options) {');
   parserFn('this.packet = packet;');
   if (options.rowsAsArray) {
     parserFn(`const result = new Array(${fields.length});`);
   } else {
-    parserFn('const result = {};');
+    parserFn('const result = Object.create(null);');
+    parserFn(`Object.defineProperty(result, "constructor", {
+      value: Object.create(null),
+      writable: false,
+      configurable: false,
+      enumerable: false
+    });`);
   }
 
   const resultTables = {};
   let resultTablesArray = [];
 
@@ -144,11 +147,13 @@ function compile(fields, options, config) {
     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++) {
-      parserFn(`result[${helpers.srcEscape(resultTablesArray[i])}] = {};`);
+      parserFn(
+        `result[${helpers.srcEscape(resultTablesArray[i])}] = Object.create(null);`,
+      );
     }
   }
 
   let lvalue = '';
   let fieldName = '';
@@ -189,14 +194,10 @@ function compile(fields, options, config) {
 
   parserFn('return result;');
   parserFn('}');
   parserFn('};')('})()');
 
-  /* eslint-enable no-trailing-spaces */
-  /* eslint-enable no-spaced-func */
-  /* eslint-enable no-unexpected-multiline */
-
   if (config.debug) {
     helpers.printDebugWithCode(
       'Compiled text protocol row parser',
       parserFn.toString(),
     );
diff --git a/test/common.test.cjs b/test/common.test.cjs
index 88b37502..e1ffa972 100644
--- a/test/common.test.cjs
+++ b/test/common.test.cjs
@@ -103,10 +103,11 @@ exports.createConnection = function (args) {
     dateStrings: args && args.dateStrings,
     authSwitchHandler: args && args.authSwitchHandler,
     typeCast: args && args.typeCast,
     namedPlaceholders: args && args.namedPlaceholders,
     connectTimeout: args && args.connectTimeout,
+    nestTables: args && args.nestTables,
     ssl: (args && args.ssl) ?? config.ssl,
   };
 
   const conn = driver.createConnection(params);
   return conn;
diff --git a/test/esm/unit/parsers/prototype-binary-results.test.mjs b/test/esm/unit/parsers/prototype-binary-results.test.mjs
new file mode 100644
index 00000000..a33eec07
--- /dev/null
+++ b/test/esm/unit/parsers/prototype-binary-results.test.mjs
@@ -0,0 +1,72 @@
+import { test, describe, assert } from 'poku';
+import { createConnection, describeOptions } from '../../../common.test.cjs';
+
+const connection = createConnection().promise();
+
+describe('Binary Parser: Prototype Sanitization', describeOptions);
+
+Promise.all([
+  test(async () => {
+    const expected = [{}];
+    expected[0].test = 2;
+
+    const [results] = await connection.query('SELECT 1+1 AS `test`');
+
+    assert.notDeepStrictEqual(
+      results,
+      expected,
+      `Ensure "results" doesn't contain a standard object ({})`,
+    );
+  }),
+  test(async () => {
+    const expected = [Object.create(null)];
+    expected[0].test = 2;
+
+    const [results] = await connection.execute('SELECT 1+1 AS `test`');
+
+    assert.deepStrictEqual(results, expected, 'Ensure clean object "results"');
+    assert.strictEqual(
+      Object.getPrototypeOf(results[0]),
+      null,
+      'Ensure clean properties in results items',
+    );
+    assert.strictEqual(
+      typeof results[0].toString,
+      'undefined',
+      'Re-check prototypes (manually) in results columns',
+    );
+    assert.strictEqual(
+      typeof results[0].test.toString,
+      'function',
+      'Ensure that the end-user is able to use prototypes',
+    );
+    assert.strictEqual(
+      results[0].test.toString(),
+      '2',
+      'Ensure that the end-user is able to use prototypes (manually): toString',
+    );
+    assert.strictEqual(
+      results[0].test.toFixed(2),
+      '2.00',
+      'Ensure that the end-user is able to use prototypes (manually): toFixed',
+    );
+
+    results[0].customProp = true;
+    assert.strictEqual(
+      results[0].customProp,
+      true,
+      'Ensure that the end-user is able to use custom props',
+    );
+  }),
+  test(async () => {
+    const [result] = await connection.execute('SET @1 = 1;');
+
+    assert.strictEqual(
+      result.constructor.name,
+      'ResultSetHeader',
+      'Ensure constructor name in result object',
+    );
+  }),
+]).then(async () => {
+  await connection.end();
+});
diff --git a/test/esm/unit/parsers/prototype-text-results.test.mjs b/test/esm/unit/parsers/prototype-text-results.test.mjs
new file mode 100644
index 00000000..99a6a693
--- /dev/null
+++ b/test/esm/unit/parsers/prototype-text-results.test.mjs
@@ -0,0 +1,72 @@
+import { test, describe, assert } from 'poku';
+import { createConnection, describeOptions } from '../../../common.test.cjs';
+
+const connection = createConnection().promise();
+
+describe('Text Parser: Prototype Sanitization', describeOptions);
+
+Promise.all([
+  test(async () => {
+    const expected = [{}];
+    expected[0].test = 2;
+
+    const [results] = await connection.query('SELECT 1+1 AS `test`');
+
+    assert.notDeepStrictEqual(
+      results,
+      expected,
+      `Ensure "results" doesn't contain a standard object ({})`,
+    );
+  }),
+  test(async () => {
+    const expected = [Object.create(null)];
+    expected[0].test = 2;
+
+    const [results] = await connection.query('SELECT 1+1 AS `test`');
+
+    assert.deepStrictEqual(results, expected, 'Ensure clean object "results"');
+    assert.strictEqual(
+      Object.getPrototypeOf(results[0]),
+      null,
+      'Ensure clean properties in results items',
+    );
+    assert.strictEqual(
+      typeof results[0].toString,
+      'undefined',
+      'Re-check prototypes (manually) in results columns',
+    );
+    assert.strictEqual(
+      typeof results[0].test.toString,
+      'function',
+      'Ensure that the end-user is able to use prototypes',
+    );
+    assert.strictEqual(
+      results[0].test.toString(),
+      '2',
+      'Ensure that the end-user is able to use prototypes (manually): toString',
+    );
+    assert.strictEqual(
+      results[0].test.toFixed(2),
+      '2.00',
+      'Ensure that the end-user is able to use prototypes (manually): toFixed',
+    );
+
+    results[0].customProp = true;
+    assert.strictEqual(
+      results[0].customProp,
+      true,
+      'Ensure that the end-user is able to use custom props',
+    );
+  }),
+  test(async () => {
+    const [result] = await connection.query('SET @1 = 1;');
+
+    assert.strictEqual(
+      result.constructor.name,
+      'ResultSetHeader',
+      'Ensure constructor name in result object',
+    );
+  }),
+]).then(async () => {
+  await connection.end();
+});

Vulnerable code:   "functions": 78,
    parserFn('const result = {};');
      parserFn(`if (!result[${tableName}]) result[${tableName}] = {};`);
  /* eslint-disable no-trailing-spaces */
  /* eslint-disable no-spaced-func */
  /* eslint-disable no-unexpected-multiline */
    parserFn('const result = {};');
      parserFn(`result[${helpers.srcEscape(resultTablesArray[i])}] = {};`);
  /* eslint-enable no-trailing-spaces */
  /* eslint-enable no-spaced-func */
  /* eslint-enable no-unexpected-multiline */

Fixed code:   "functions": 77,
    parserFn('const result = Object.create(null);');
    parserFn(`Object.defineProperty(result, "constructor", {
      value: Object.create(null),
      writable: false,
      configurable: false,
      enumerable: false
    });`);
      parserFn(
        `if (!result[${tableName}]) result[${tableName}] = Object.create(null);`,
      );
    parserFn('const result = Object.create(null);');
    parserFn(`Object.defineProperty(result, "constructor", {
      value: Object.create(null),
      writable: false,
      configurable: false,
      enumerable: false
    });`);
      parserFn(
        `result[${helpers.srcEscape(resultTablesArray[i])}] = Object.create(null);`,
      );
    nestTables: args && args.nestTables,
import { test, describe, assert } from 'poku';
import { createConnection, describeOptions } from '../../../common.test.cjs';

const connection = createConnection().promise();

describe('Binary Parser: Prototype Sanitization', describeOptions);

Promise.all([
  test(async () => {
    const expected = [{}];
    expected[0].test = 2;

    const [results] = await connection.query('SELECT 1+1 AS `test`');

    assert.notDeepStrictEqual(
      results,
      expected,
      `Ensure "results" doesn't contain a standard object ({})`,
    );
  }),
  test(async () => {
    const expected = [Object.create(null)];
    expected[0].test = 2;

    const [results] = await connection.execute('SELECT 1+1 AS `test`');

    assert.deepStrictEqual(results, expected, 'Ensure clean object "results"');
    assert.strictEqual(
      Object.getPrototypeOf(results[0]),
      null,
      'Ensure clean properties in results items',
    );
    assert.strictEqual(
      typeof results[0].toString,
      'undefined',
      'Re-check prototypes (manually) in results columns',
    );
    assert.strictEqual(
      typeof results[0].test.toString,
      'function',
      'Ensure that the end-user is able to use prototypes',
    );
    assert.strictEqual(
      results[0].test.toString(),
      '2',
      'Ensure that the end-user is able to use prototypes (manually): toString',
    );
    assert.strictEqual(
      results[0].test.toFixed(2),
      '2.00',
      'Ensure that the end-user is able to use prototypes (manually): toFixed',
    );

    results[0].customProp = true;
    assert.strictEqual(
      results[0].customProp,
      true,
      'Ensure that the end-user is able to use custom props',
    );
  }),
  test(async () => {
    const [result] = await connection.execute('SET @1 = 1;');

    assert.strictEqual(
      result.constructor.name,
      'ResultSetHeader',
      'Ensure constructor name in result object',
    );
  }),
]).then(async () => {
  await connection.end();
});
import { test, describe, assert } from 'poku';
import { createConnection, describeOptions } from '../../../common.test.cjs';

const connection = createConnection().promise();

describe('Text Parser: Prototype Sanitization', describeOptions);

Promise.all([
  test(async () => {
    const expected = [{}];
    expected[0].test = 2;

    const [results] = await connection.query('SELECT 1+1 AS `test`');

    assert.notDeepStrictEqual(
      results,
      expected,
      `Ensure "results" doesn't contain a standard object ({})`,
    );
  }),
  test(async () => {
    const expected = [Object.create(null)];
    expected[0].test = 2;

    const [results] = await connection.query('SELECT 1+1 AS `test`');

    assert.deepStrictEqual(results, expected, 'Ensure clean object "results"');
    assert.strictEqual(
      Object.getPrototypeOf(results[0]),
      null,
      'Ensure clean properties in results items',
    );
    assert.strictEqual(
      typeof results[0].toString,
      'undefined',
      'Re-check prototypes (manually) in results columns',
    );
    assert.strictEqual(
      typeof results[0].test.toString,
      'function',
      'Ensure that the end-user is able to use prototypes',
    );
    assert.strictEqual(
      results[0].test.toString(),
      '2',
      'Ensure that the end-user is able to use prototypes (manually): toString',
    );
    assert.strictEqual(
      results[0].test.toFixed(2),
      '2.00',
      'Ensure that the end-user is able to use prototypes (manually): toFixed',
    );

    results[0].customProp = true;
    assert.strictEqual(
      results[0].customProp,
      true,
      'Ensure that the end-user is able to use custom props',
    );
  }),
  test(async () => {
    const [result] = await connection.query('SET @1 = 1;');

    assert.strictEqual(
      result.constructor.name,
      'ResultSetHeader',
      'Ensure constructor name in result object',
    );
  }),
]).then(async () => {
  await connection.end();
});