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`

lib/helpers.js

@@ -73,3 +73,15 @@ const privateObjectProps = new Set([
 ]);
 
 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;

lib/parsers/binary_parser.js

@@ -145,22 +145,14 @@ function compile(fields, options, config) {
  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) {
@@ -190,7 +182,6 @@ function compile(fields, options, config) {
  }
  parserFn('}');
 
-
  currentFieldNullBit *= 2;
  if (currentFieldNullBit === 0x100) {
  currentFieldNullBit = 1;

lib/parsers/text_parser.js

@@ -143,28 +143,24 @@ function compile(fields, options, config) {
  }
  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 {

test/esm/unit/parsers/ensure-safe-binary-fields.test.mjs

@@ -1,13 +1,12 @@
 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) => {
@@ -17,8 +16,42 @@ blockedFields.forEach((fields) => {
  } 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`,
+ );
+ }
+ });

test/esm/unit/parsers/ensure-safe-text-fields.test.mjs

@@ -1,13 +1,12 @@
 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) => {
@@ -17,8 +16,42 @@ blockedFields.forEach((fields) => {
  } 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`,
+ );
+ }
+ });