Security: SQL injection risk from unvalidated ORDER BY direction and field tokens

packages/winnow/src/sql-query-builder/order-by.js

@@ -6,11 +6,17 @@ function createOrderByClause(options = {}) {
 
   const orderByClause = orderByArray
     .map((orderBy) => {
-      const [field, direction = 'ASC'] = orderBy.split(' ');
+      const [field, direction = 'ASC', ...extraTokens] = orderBy.trim().split(/\s+/);
+      if (!isValidOrderByField(field) || extraTokens.length || !isValidOrderByDirection(direction)) {
+        throw new Error('Invalid order by clause');
+      }
+
+      const normalizedDirection = direction.toUpperCase();
+
       if (shouldFormatForAggregationQuery(field, aggregates)) {
-        return `\`${field}\` ${direction.toUpperCase()}`;
+        return `\`${field}\` ${normalizedDirection}`;
       }
-      return `${selector}->\`${field}\` ${direction.toUpperCase()}`;
+      return `${selector}->\`${field}\` ${normalizedDirection}`;
     })
     .join(', ');
 
@@ -26,4 +32,13 @@ function shouldFormatForAggregationQuery(field, aggregations) {
   );
 }
 
+function isValidOrderByDirection(direction) {
+  const normalizedDirection = direction.toUpperCase();
+  return normalizedDirection === 'ASC' || normalizedDirection === 'DESC';
+}
+
+function isValidOrderByField(field) {
+  return /^[A-Za-z0-9_]+$/.test(field);
+}
+
 module.exports = createOrderByClause;

packages/winnow/src/sql-query-builder/select/fields-select-fragment.js

@@ -10,22 +10,38 @@ function createFieldsSelectFragment(options = {}) {
 
 function selectFieldsAsEsriJson(options) {
   const { fields, dateFields = [], returnIdsOnly, idField = null } = options;
-  const delimitedDateFields = dateFields.join(',');
-  const includeIdField = shouldIncludeIdField({ returnIdsOnly, fields });
-  if (fields) {
-    return `selectFieldsToEsriAttributes(properties, geometry, "${fields.join(',')}", "${delimitedDateFields}", "${includeIdField}", "${idField}") as attributes`;  // eslint-disable-line
+  const validatedFields = validateFields(fields);
+  const validatedDateFields = validateFields(dateFields);
+  const validatedIdField = idField ? validateField(idField) : null;
+  const delimitedDateFields = validatedDateFields.join(',');
+  const includeIdField = shouldIncludeIdField({ returnIdsOnly, fields: validatedFields });
+  if (validatedFields) {
+    return `selectFieldsToEsriAttributes(properties, geometry, "${validatedFields.join(',')}", "${delimitedDateFields}", "${includeIdField}", "${validatedIdField}") as attributes`;  // eslint-disable-line
   }
-  return `toEsriAttributes(properties, geometry, "${delimitedDateFields}", "${includeIdField}", "${idField}") as attributes`;   // eslint-disable-line
+  return `toEsriAttributes(properties, geometry, "${delimitedDateFields}", "${includeIdField}", "${validatedIdField}") as attributes`;   // eslint-disable-line
 }
 
 function shouldIncludeIdField({ returnIdsOnly, fields }) {
   if (returnIdsOnly || !fields || fields.includes('OBJECTID')) return true;
   return false;
 }
 
+function validateFields(fields) {
+  if (!fields) return fields;
+  return fields.map(validateField);
+}
+
+function validateField(field) {
+  if (!/^[A-Za-z0-9_]+$/.test(field)) {
+    throw new Error('Invalid field');
+  }
+  return field;
+}
+
 function selectFieldsAsGeoJson(fields) {
-  if (fields) {
-    return `selectFields(properties, "${fields.join(',')}") as properties`;
+  const validatedFields = validateFields(fields);
+  if (validatedFields) {
+    return `selectFields(properties, "${validatedFields.join(',')}") as properties`;
   }
   return 'type, properties as properties';
 }