CVE-2024-21512: Insufficient Information (NVD-CWE-noinfo)
value: Versions of the package mysql2 before 3.9.8 are vulnerable to Prototype Pollution due to improper user input sanitization passed to fields and tables when using nestTables.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
Published: 2024-05-29T05:16Z
Repository: https://github.com/sidorares/node-mysql2
Language: JavaScript
Files changed: lib/helpers.js, lib/parsers/binary_parser.js, lib/parsers/text_parser.js, test/esm/unit/parsers/ensure-safe-binary-fields.test.mjs, test/esm/unit/parsers/ensure-safe-text-fields.test.mjs
Commit date: 2024-05-27 09:51:25 +1000
Commit message: fix(security): sanitize fields and tables when using nestTables (#2702)

* fix(security): sanitize fields and tables when using nestTables

* chore: undone `nestTables` validation for `rowsAsArray`
Diff: diff --git a/lib/helpers.js b/lib/helpers.js
index fdba2ddb7..55a52bb46 100644
--- a/lib/helpers.js
+++ b/lib/helpers.js
@@ -71,5 +71,17 @@ const privateObjectProps = new Set([
   '__lookupSetter__',
   '__proto__',
 ]);
 
 exports.privateObjectProps = privateObjectProps;
+
+const fieldEscape = (field) => {
+  if (privateObjectProps.has(field)) {
+    throw new Error(
+      `The field name (${field}) can't be the same as an object's private property.`,
+    );
+  }
+
+  return srcEscape(field);
+};
+
+exports.fieldEscape = fieldEscape;
diff --git a/lib/parsers/binary_parser.js b/lib/parsers/binary_parser.js
index 9d53d8c18..59d87bca3 100644
--- a/lib/parsers/binary_parser.js
+++ b/lib/parsers/binary_parser.js
@@ -143,26 +143,18 @@ function compile(fields, options, config) {
   let nullByteIndex = 0;
   let fieldName = '';
   let tableName = '';
 
   for (let i = 0; i < fields.length; i++) {
-    fieldName = helpers.srcEscape(fields[i].name);
-
-    if (helpers.privateObjectProps.has(fields[i].name)) {
-      throw new Error(
-        `The field name (${fieldName}) can't be the same as an object's private property.`,
-      );
-    }
-
-    parserFn(`// ${fieldName}: ${typeNames[fields[i].columnType]}`);
+    fieldName = helpers.fieldEscape(fields[i].name);
+    // parserFn(`// ${fieldName}: ${typeNames[fields[i].columnType]}`);
 
     if (typeof options.nestTables === 'string') {
-      lvalue = `result[${helpers.srcEscape(
-        fields[i].table + options.nestTables + fields[i].name,
-      )}]`;
+      lvalue = `result[${helpers.fieldEscape(fields[i].table + options.nestTables + fields[i].name)}]`;
     } else if (options.nestTables === true) {
-      tableName = helpers.srcEscape(fields[i].table);
+      tableName = helpers.fieldEscape(fields[i].table);
+
       parserFn(`if (!result[${tableName}]) result[${tableName}] = {};`);
       lvalue = `result[${tableName}][${fieldName}]`;
     } else if (options.rowsAsArray) {
       lvalue = `result[${i.toString(10)}]`;
     } else {
@@ -188,11 +180,10 @@ function compile(fields, options, config) {
         parserFn(`${lvalue} = ${readCode};`);
       }
     }
     parserFn('}');
 
-
     currentFieldNullBit *= 2;
     if (currentFieldNullBit === 0x100) {
       currentFieldNullBit = 1;
       nullByteIndex++;
     }
diff --git a/lib/parsers/text_parser.js b/lib/parsers/text_parser.js
index be89e7712..acd75caca 100644
--- a/lib/parsers/text_parser.js
+++ b/lib/parsers/text_parser.js
@@ -141,32 +141,28 @@ 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.fieldEscape(resultTablesArray[i])}] = {};`);
     }
   }
 
   let lvalue = '';
   let fieldName = '';
+  let tableName = '';
   for (let i = 0; i < fields.length; i++) {
-    fieldName = helpers.srcEscape(fields[i].name);
+    fieldName = helpers.fieldEscape(fields[i].name);
+    // parserFn(`// ${fieldName}: ${typeNames[fields[i].columnType]}`);
 
-    if (helpers.privateObjectProps.has(fields[i].name)) {
-      throw new Error(
-        `The field name (${fieldName}) can't be the same as an object's private property.`,
-      );
-    }
-
-    parserFn(`// ${fieldName}: ${typeNames[fields[i].columnType]}`);
     if (typeof options.nestTables === 'string') {
-      lvalue = `result[${helpers.srcEscape(
-        fields[i].table + options.nestTables + fields[i].name,
-      )}]`;
+      lvalue = `result[${helpers.fieldEscape(fields[i].table + options.nestTables + fields[i].name)}]`;
     } else if (options.nestTables === true) {
-      lvalue = `result[${helpers.srcEscape(fields[i].table)}][${fieldName}]`;
+      tableName = helpers.fieldEscape(fields[i].table);
+
+      parserFn(`if (!result[${tableName}]) result[${tableName}] = {};`);
+      lvalue = `result[${tableName}][${fieldName}]`;
     } else if (options.rowsAsArray) {
       lvalue = `result[${i.toString(10)}]`;
     } else {
       lvalue = `result[${fieldName}]`;
     }
diff --git a/test/esm/unit/parsers/ensure-safe-binary-fields.test.mjs b/test/esm/unit/parsers/ensure-safe-binary-fields.test.mjs
index a16cc4301..3cce43c1e 100644
--- a/test/esm/unit/parsers/ensure-safe-binary-fields.test.mjs
+++ b/test/esm/unit/parsers/ensure-safe-binary-fields.test.mjs
@@ -1,24 +1,57 @@
 import { describe, assert } from 'poku';
 import { describeOptions } from '../../../common.test.cjs';
 import getBinaryParser from '../../../../lib/parsers/binary_parser.js';
-import { srcEscape } from '../../../../lib/helpers.js';
 import { privateObjectProps } from '../../../../lib/helpers.js';
 
 describe('Binary Parser: Block Native Object Props', describeOptions);
 
 const blockedFields = Array.from(privateObjectProps).map((prop) => [
-  { name: prop },
+  { name: prop, table: '' },
 ]);
 
 blockedFields.forEach((fields) => {
   try {
     getBinaryParser(fields, {}, {});
     assert.fail('An error was expected');
   } catch (error) {
     assert.strictEqual(
       error.message,
-      `The field name (${srcEscape(fields[0].name)}) can't be the same as an object's private property.`,
+      `The field name (${fields[0].name}) can't be the same as an object's private property.`,
       `Ensure safe ${fields[0].name}`,
     );
   }
 });
+
+blockedFields
+  .map((fields) =>
+    fields.map((field) => ({ ...field, name: field.name.slice(1) })),
+  )
+  .forEach((fields) => {
+    try {
+      getBinaryParser(fields, { nestTables: '_' }, {});
+      assert.fail('An error was expected');
+    } catch (error) {
+      assert.strictEqual(
+        error.message,
+        `The field name (_${fields[0].name}) can't be the same as an object's private property.`,
+        `Ensure safe _${fields[0].name} for nestTables as string`,
+      );
+    }
+  });
+
+blockedFields
+  .map((fields) =>
+    fields.map((field) => ({ ...field, name: '', table: field.name })),
+  )
+  .forEach((fields) => {
+    try {
+      getBinaryParser(fields, { nestTables: true }, {});
+      assert.fail('An error was expected');
+    } catch (error) {
+      assert.strictEqual(
+        error.message,
+        `The field name (${fields[0].table}) can't be the same as an object's private property.`,
+        `Ensure safe ${fields[0].table} for nestTables as true`,
+      );
+    }
+  });
diff --git a/test/esm/unit/parsers/ensure-safe-text-fields.test.mjs b/test/esm/unit/parsers/ensure-safe-text-fields.test.mjs
index 36ef7f451..2ea4b04b7 100644
--- a/test/esm/unit/parsers/ensure-safe-text-fields.test.mjs
+++ b/test/esm/unit/parsers/ensure-safe-text-fields.test.mjs
@@ -1,24 +1,57 @@
 import { describe, assert } from 'poku';
 import { describeOptions } from '../../../common.test.cjs';
 import TextRowParser from '../../../../lib/parsers/text_parser.js';
-import { srcEscape } from '../../../../lib/helpers.js';
 import { privateObjectProps } from '../../../../lib/helpers.js';
 
 describe('Text Parser: Block Native Object Props', describeOptions);
 
 const blockedFields = Array.from(privateObjectProps).map((prop) => [
-  { name: prop },
+  { name: prop, table: '' },
 ]);
 
 blockedFields.forEach((fields) => {
   try {
     TextRowParser(fields, {}, {});
     assert.fail('An error was expected');
   } catch (error) {
     assert.strictEqual(
       error.message,
-      `The field name (${srcEscape(fields[0].name)}) can't be the same as an object's private property.`,
+      `The field name (${fields[0].name}) can't be the same as an object's private property.`,
       `Ensure safe ${fields[0].name}`,
     );
   }
 });
+
+blockedFields
+  .map((fields) =>
+    fields.map((field) => ({ ...field, name: field.name.slice(1) })),
+  )
+  .forEach((fields) => {
+    try {
+      TextRowParser(fields, { nestTables: '_' }, {});
+      assert.fail('An error was expected');
+    } catch (error) {
+      assert.strictEqual(
+        error.message,
+        `The field name (_${fields[0].name}) can't be the same as an object's private property.`,
+        `Ensure safe _${fields[0].name} for nestTables as string`,
+      );
+    }
+  });
+
+blockedFields
+  .map((fields) =>
+    fields.map((field) => ({ ...field, name: '', table: field.name })),
+  )
+  .forEach((fields) => {
+    try {
+      TextRowParser(fields, { nestTables: true }, {});
+      assert.fail('An error was expected');
+    } catch (error) {
+      assert.strictEqual(
+        error.message,
+        `The field name (${fields[0].table}) can't be the same as an object's private property.`,
+        `Ensure safe ${fields[0].table} for nestTables as true`,
+      );
+    }
+  });

Vulnerable code:     fieldName = helpers.srcEscape(fields[i].name);

    if (helpers.privateObjectProps.has(fields[i].name)) {
      throw new Error(
        `The field name (${fieldName}) can't be the same as an object's private property.`,
      );
    }

    parserFn(`// ${fieldName}: ${typeNames[fields[i].columnType]}`);
      lvalue = `result[${helpers.srcEscape(
        fields[i].table + options.nestTables + fields[i].name,
      )}]`;
      tableName = helpers.srcEscape(fields[i].table);

      parserFn(`result[${helpers.srcEscape(resultTablesArray[i])}] = {};`);
    fieldName = helpers.srcEscape(fields[i].name);
    if (helpers.privateObjectProps.has(fields[i].name)) {
      throw new Error(
        `The field name (${fieldName}) can't be the same as an object's private property.`,
      );
    }

    parserFn(`// ${fieldName}: ${typeNames[fields[i].columnType]}`);
      lvalue = `result[${helpers.srcEscape(
        fields[i].table + options.nestTables + fields[i].name,
      )}]`;
      lvalue = `result[${helpers.srcEscape(fields[i].table)}][${fieldName}]`;
import { srcEscape } from '../../../../lib/helpers.js';
  { name: prop },
      `The field name (${srcEscape(fields[0].name)}) can't be the same as an object's private property.`,
import { srcEscape } from '../../../../lib/helpers.js';
  { name: prop },
      `The field name (${srcEscape(fields[0].name)}) can't be the same as an object's private property.`,
Fixed code: 
const fieldEscape = (field) => {
  if (privateObjectProps.has(field)) {
    throw new Error(
      `The field name (${field}) can't be the same as an object's private property.`,
    );
  }

  return srcEscape(field);
};

exports.fieldEscape = fieldEscape;
    fieldName = helpers.fieldEscape(fields[i].name);
    // parserFn(`// ${fieldName}: ${typeNames[fields[i].columnType]}`);
      lvalue = `result[${helpers.fieldEscape(fields[i].table + options.nestTables + fields[i].name)}]`;
      tableName = helpers.fieldEscape(fields[i].table);

      parserFn(`result[${helpers.fieldEscape(resultTablesArray[i])}] = {};`);
  let tableName = '';
    fieldName = helpers.fieldEscape(fields[i].name);
    // parserFn(`// ${fieldName}: ${typeNames[fields[i].columnType]}`);
      lvalue = `result[${helpers.fieldEscape(fields[i].table + options.nestTables + fields[i].name)}]`;
      tableName = helpers.fieldEscape(fields[i].table);

      parserFn(`if (!result[${tableName}]) result[${tableName}] = {};`);
      lvalue = `result[${tableName}][${fieldName}]`;
  { name: prop, table: '' },
      `The field name (${fields[0].name}) can't be the same as an object's private property.`,

blockedFields
  .map((fields) =>
    fields.map((field) => ({ ...field, name: field.name.slice(1) })),
  )
  .forEach((fields) => {
    try {
      getBinaryParser(fields, { nestTables: '_' }, {});
      assert.fail('An error was expected');
    } catch (error) {
      assert.strictEqual(
        error.message,
        `The field name (_${fields[0].name}) can't be the same as an object's private property.`,
        `Ensure safe _${fields[0].name} for nestTables as string`,
      );
    }
  });

blockedFields
  .map((fields) =>
    fields.map((field) => ({ ...field, name: '', table: field.name })),
  )
  .forEach((fields) => {
    try {
      getBinaryParser(fields, { nestTables: true }, {});
      assert.fail('An error was expected');
    } catch (error) {
      assert.strictEqual(
        error.message,
        `The field name (${fields[0].table}) can't be the same as an object's private property.`,
        `Ensure safe ${fields[0].table} for nestTables as true`,
      );
    }
  });
  { name: prop, table: '' },
      `The field name (${fields[0].name}) can't be the same as an object's private property.`,

blockedFields
  .map((fields) =>
    fields.map((field) => ({ ...field, name: field.name.slice(1) })),
  )
  .forEach((fields) => {
    try {
      TextRowParser(fields, { nestTables: '_' }, {});
      assert.fail('An error was expected');
    } catch (error) {
      assert.strictEqual(
        error.message,
        `The field name (_${fields[0].name}) can't be the same as an object's private property.`,
        `Ensure safe _${fields[0].name} for nestTables as string`,
      );
    }
  });

blockedFields
  .map((fields) =>
    fields.map((field) => ({ ...field, name: '', table: field.name })),
  )
  .forEach((fields) => {
    try {
      TextRowParser(fields, { nestTables: true }, {});
      assert.fail('An error was expected');
    } catch (error) {
      assert.strictEqual(
        error.message,
        `The field name (${fields[0].table}) can't be the same as an object's private property.`,
        `Ensure safe ${fields[0].table} for nestTables as true`,
      );
    }
  });