From 6ba2e17ec4e8e6db150e18dcea39eea574a99f1c Mon Sep 17 00:00:00 2001 From: tuanaiseo Date: Sun, 5 Apr 2026 18:28:34 +0700 Subject: [PATCH 1/2] fix(security)(winnow): sql injection risk from unvalidated order by direc `order-by.js` splits each order clause by space and directly interpolates both `field` and `direction` into SQL. `direction` is only uppercased, not validated to `ASC|DESC`, and `field` is not escaped for delimiter-breaking characters. A crafted `orderByFields` value can inject additional SQL or malformed expressions. Affected files: order-by.js, fields-select-fragment.js Signed-off-by: tuanaiseo <221258316+tuanaiseo@users.noreply.github.com> --- .../winnow/src/sql-query-builder/order-by.js | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/packages/winnow/src/sql-query-builder/order-by.js b/packages/winnow/src/sql-query-builder/order-by.js index e98ef72a..5a4dea52 100644 --- a/packages/winnow/src/sql-query-builder/order-by.js +++ b/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; From eee0b323b53c157b8d86bb81f401373fda4df731 Mon Sep 17 00:00:00 2001 From: tuanaiseo Date: Sun, 5 Apr 2026 18:28:34 +0700 Subject: [PATCH 2/2] fix(security)(winnow): sql injection risk from unvalidated order by direc `order-by.js` splits each order clause by space and directly interpolates both `field` and `direction` into SQL. `direction` is only uppercased, not validated to `ASC|DESC`, and `field` is not escaped for delimiter-breaking characters. A crafted `orderByFields` value can inject additional SQL or malformed expressions. Affected files: order-by.js, fields-select-fragment.js Signed-off-by: tuanaiseo <221258316+tuanaiseo@users.noreply.github.com> --- .../select/fields-select-fragment.js | 30 ++++++++++++++----- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/packages/winnow/src/sql-query-builder/select/fields-select-fragment.js b/packages/winnow/src/sql-query-builder/select/fields-select-fragment.js index 6ce993d4..0878b485 100644 --- a/packages/winnow/src/sql-query-builder/select/fields-select-fragment.js +++ b/packages/winnow/src/sql-query-builder/select/fields-select-fragment.js @@ -10,12 +10,15 @@ 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 }) { @@ -23,9 +26,22 @@ function shouldIncludeIdField({ returnIdsOnly, fields }) { 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'; }