Merge branch 'main' into releases/4.11.0

api-tests/core/strapi/api/sanitize/sanitize-query.test.api.js

@@ -135,6 +135,28 @@ describe('Core API - Sanitize', () => {
           checkAPIResultLength(res, 0);
         });
 
+        it('Successfully filters invalid attributes', async () => {
+          const document = data.document[2];
+          const filters = {
+            ID: document.id, // invalid casing on key 'id'
+            notAnAttribute: '', // doesn't exist on schema
+            t0: { createdBy: { id: { $lt: '1' } } }, // join table name
+            t1: { createdBy: { id: { $lt: '1' } } }, // join table name
+            t0: { updatedBy: { id: { $lt: '1' } } }, // join table name
+            t1: { updatedBy: { id: { $lt: '1' } } }, // join table name
+            $fakeOp: false,
+            id: { $STARTSWITH: '123' }, // wrong casing for operator
+          };
+
+          const res = await rq.get('/api/documents', { qs: { filters } });
+
+          // Should not return a 500 error from notAnAttribute or $fakeOp
+          expect(res.status).toEqual(200);
+
+          // Should receive all documents because createdBy was filtered out
+          checkAPIResultLength(res, documentsLength());
+        });
+
         it('Successfully filters on valid ID', async () => {
           const document = data.document[2];
           const res = await rq.get('/api/documents', { qs: { filters: { id: document.id } } });

packages/core/database/lib/query/helpers/where.js

@@ -1,55 +1,14 @@
 'use strict';
 
-const _ = require('lodash/fp');
+const { isArray, castArray, keys, isPlainObject } = require('lodash/fp');
 
+const { isOperatorOfType } = require('@strapi/utils');
 const types = require('../../types');
 const { createField } = require('../../fields');
 const { createJoin } = require('./join');
 const { toColumnName } = require('./transform');
 const { isKnexQuery } = require('../../utils/knex');
 
-const GROUP_OPERATORS = ['$and', '$or'];
-const OPERATORS = [
-  '$not',
-  '$in',
-  '$notIn',
-  '$eq',
-  '$eqi',
-  '$ne',
-  '$gt',
-  '$gte',
-  '$lt',
-  '$lte',
-  '$null',
-  '$notNull',
-  '$between',
-  '$startsWith',
-  '$endsWith',
-  '$startsWithi',
-  '$endsWithi',
-  '$contains',
-  '$notContains',
-  '$containsi',
-  '$notContainsi',
-];
-
-const CAST_OPERATORS = [
-  '$not',
-  '$in',
-  '$notIn',
-  '$eq',
-  '$ne',
-  '$gt',
-  '$gte',
-  '$lt',
-  '$lte',
-  '$between',
-];
-
-const ARRAY_OPERATORS = ['$in', '$notIn', '$between'];
-
-const isOperator = (key) => OPERATORS.includes(key);
-
 const castValue = (value, attribute) => {
   if (!attribute) {
     return value;
@@ -65,12 +24,12 @@ const castValue = (value, attribute) => {
 };
 
 const processAttributeWhere = (attribute, where, operator = '$eq') => {
-  if (_.isArray(where)) {
+  if (isArray(where)) {
     return where.map((sub) => processAttributeWhere(attribute, sub, operator));
   }
 
-  if (!_.isPlainObject(where)) {
+  if (!isPlainObject(where)) {
-    if (CAST_OPERATORS.includes(operator)) {
+    if (isOperatorOfType('cast', operator)) {
       return castValue(where, attribute);
     }
 
@@ -82,7 +41,7 @@ const processAttributeWhere = (attribute, where, operator = '$eq') => {
   for (const key of Object.keys(where)) {
     const value = where[key];
 
-    if (!isOperator(key)) {
+    if (!isOperatorOfType('where', key)) {
       throw new Error(`Undefined attribute level operator ${key}`);
     }
 
@@ -100,16 +59,16 @@ const processAttributeWhere = (attribute, where, operator = '$eq') => {
  * @returns {Object}
  */
 const processWhere = (where, ctx) => {
-  if (!_.isArray(where) && !_.isPlainObject(where)) {
+  if (!isArray(where) && !isPlainObject(where)) {
     throw new Error('Where must be an array or an object');
   }
 
-  if (_.isArray(where)) {
+  if (isArray(where)) {
     return where.map((sub) => processWhere(sub, ctx));
   }
 
   const processNested = (where, ctx) => {
-    if (!_.isPlainObject(where)) {
+    if (!isPlainObject(where)) {
       return where;
     }
 
@@ -126,7 +85,7 @@ const processWhere = (where, ctx) => {
     const value = where[key];
 
     // if operator $and $or then loop over them
-    if (GROUP_OPERATORS.includes(key)) {
+    if (isOperatorOfType('group', key)) {
       filters[key] = value.map((sub) => processNested(sub, ctx));
       continue;
     }
@@ -136,7 +95,7 @@ const processWhere = (where, ctx) => {
       continue;
     }
 
-    if (isOperator(key)) {
+    if (isOperatorOfType('where', key)) {
       throw new Error(
         `Only $and, $or and $not can only be used as root level operators. Found ${key}.`
       );
@@ -165,7 +124,7 @@ const processWhere = (where, ctx) => {
         uid: attribute.target,
       });
 
-      if (!_.isPlainObject(nestedWhere) || isOperator(_.keys(nestedWhere)[0])) {
+      if (!isPlainObject(nestedWhere) || isOperatorOfType('where', keys(nestedWhere)[0])) {
         nestedWhere = { [qb.aliasColumn('id', subAlias)]: nestedWhere };
       }
 
@@ -192,7 +151,7 @@ const processWhere = (where, ctx) => {
 
 // TODO: add type casting per operator at some point
 const applyOperator = (qb, column, operator, value) => {
-  if (Array.isArray(value) && !ARRAY_OPERATORS.includes(operator)) {
+  if (Array.isArray(value) && !isOperatorOfType('array', operator)) {
     return qb.where((subQB) => {
       value.forEach((subValue) =>
         subQB.orWhere((innerQB) => {
@@ -209,12 +168,12 @@ const applyOperator = (qb, column, operator, value) => {
     }
 
     case '$in': {
-      qb.whereIn(column, isKnexQuery(value) ? value : _.castArray(value));
+      qb.whereIn(column, isKnexQuery(value) ? value : castArray(value));
       break;
     }
 
     case '$notIn': {
-      qb.whereNotIn(column, isKnexQuery(value) ? value : _.castArray(value));
+      qb.whereNotIn(column, isKnexQuery(value) ? value : castArray(value));
       break;
     }
 
@@ -328,7 +287,7 @@ const applyOperator = (qb, column, operator, value) => {
 };
 
 const applyWhereToColumn = (qb, column, columnWhere) => {
-  if (!_.isPlainObject(columnWhere)) {
+  if (!isPlainObject(columnWhere)) {
     if (Array.isArray(columnWhere)) {
       return qb.whereIn(column, columnWhere);
     }
@@ -344,11 +303,11 @@ const applyWhereToColumn = (qb, column, columnWhere) => {
 };
 
 const applyWhere = (qb, where) => {
-  if (!_.isArray(where) && !_.isPlainObject(where)) {
+  if (!isArray(where) && !isPlainObject(where)) {
     throw new Error('Where must be an array or an object');
   }
 
-  if (_.isArray(where)) {
+  if (isArray(where)) {
     return qb.where((subQB) => where.forEach((subWhere) => applyWhere(subQB, subWhere)));
   }
 

packages/core/database/package.json

@@ -33,6 +33,7 @@
     "lint": "run -T eslint ."
   },
   "dependencies": {
+    "@strapi/utils": "4.10.7",
     "date-fns": "2.30.0",
     "debug": "4.3.4",
     "fs-extra": "10.0.0",

packages/core/utils/lib/convert-query-params.js

@@ -18,6 +18,7 @@ const {
   cloneDeep,
   get,
   mergeAll,
+  isString,
 } = require('lodash/fp');
 const _ = require('lodash');
 const parseType = require('./parse-type');
@@ -28,6 +29,7 @@ const {
   isDynamicZoneAttribute,
   isMorphToRelationalAttribute,
 } = require('./content-types');
+const { isOperator } = require('./operators');
 
 const { PUBLISHED_AT_ATTRIBUTE } = contentTypesUtils.constants;
 
@@ -46,7 +48,7 @@ class InvalidSortError extends Error {
 }
 
 const validateOrder = (order) => {
-  if (!['asc', 'desc'].includes(order.toLocaleLowerCase())) {
+  if (!isString(order) || !['asc', 'desc'].includes(order.toLocaleLowerCase())) {
     throw new InvalidOrderError();
   }
 };
@@ -80,6 +82,13 @@ const convertSortQueryParams = (sortQuery) => {
 };
 
 const convertSingleSortQueryParam = (sortQuery) => {
+  if (!sortQuery) {
+    return {};
+  }
+  if (!isString(sortQuery)) {
+    throw new Error('Invalid sort query');
+  }
+
   // split field and order param with default order to ascending
   const [field, order = 'asc'] = sortQuery.split(':');
 
@@ -89,6 +98,8 @@ const convertSingleSortQueryParam = (sortQuery) => {
 
   validateOrder(order);
 
+  // TODO: field should be a valid path on an object model
+
   return _.set({}, field, order);
 };
 
@@ -368,6 +379,7 @@ const convertNestedPopulate = (subPopulate, schema) => {
   return query;
 };
 
+// TODO: ensure field is valid in content types (will probably have to check strapi.contentTypes since it can be a string.path)
 const convertFieldsQueryParams = (fields, depth = 0) => {
   if (depth === 0 && fields === '*') {
     return undefined;
@@ -387,6 +399,18 @@ const convertFieldsQueryParams = (fields, depth = 0) => {
   throw new Error('Invalid fields parameter. Expected a string or an array of strings');
 };
 
+const isValidSchemaAttribute = (key, schema) => {
+  if (key === 'id') {
+    return true;
+  }
+
+  if (!schema) {
+    return false;
+  }
+
+  return Object.keys(schema.attributes).includes(key);
+};
+
 const convertFiltersQueryParams = (filters, schema) => {
   // Filters need to be either an array or an object
   // Here we're only checking for 'object' type since typeof [] => object and typeof {} => object
@@ -401,10 +425,6 @@ const convertFiltersQueryParams = (filters, schema) => {
 };
 
 const convertAndSanitizeFilters = (filters, schema) => {
-  if (!isPlainObject(filters)) {
-    return filters;
-  }
-
   if (Array.isArray(filters)) {
     return (
       filters
@@ -415,14 +435,23 @@ const convertAndSanitizeFilters = (filters, schema) => {
     );
   }
 
+  // This must come after check for Array or else arrays are not filtered
+  if (!isPlainObject(filters)) {
+    return filters;
+  }
+
   const removeOperator = (operator) => delete filters[operator];
 
   // Here, `key` can either be an operator or an attribute name
   for (const [key, value] of Object.entries(filters)) {
     const attribute = get(key, schema?.attributes);
+    const validKey = isOperator(key) || isValidSchemaAttribute(key, schema);
 
+    if (!validKey) {
+      removeOperator(key);
+    }
     // Handle attributes
-    if (attribute) {
+    else if (attribute) {
       // Relations
       if (attribute.type === 'relation') {
         filters[key] = convertAndSanitizeFilters(value, strapi.getModel(attribute.target));

packages/core/utils/lib/index.js

@@ -42,6 +42,7 @@ const importDefault = require('./import-default');
 const template = require('./template');
 const file = require('./file');
 const traverse = require('./traverse');
+const { isOperator, isOperatorOfType } = require('./operators');
 
 module.exports = {
   yup,
@@ -91,4 +92,6 @@ module.exports = {
   importDefault,
   file,
   traverse,
+  isOperator,
+  isOperatorOfType,
 };

packages/core/utils/lib/operators.js

@@ -0,0 +1,74 @@
+'use strict';
+
+const GROUP_OPERATORS = ['$and', '$or'];
+
+const WHERE_OPERATORS = [
+  '$not',
+  '$in',
+  '$notIn',
+  '$eq',
+  '$eqi',
+  '$ne',
+  '$gt',
+  '$gte',
+  '$lt',
+  '$lte',
+  '$null',
+  '$notNull',
+  '$between',
+  '$startsWith',
+  '$endsWith',
+  '$startsWithi',
+  '$endsWithi',
+  '$contains',
+  '$notContains',
+  '$containsi',
+  '$notContainsi',
+];
+
+const CAST_OPERATORS = [
+  '$not',
+  '$in',
+  '$notIn',
+  '$eq',
+  '$ne',
+  '$gt',
+  '$gte',
+  '$lt',
+  '$lte',
+  '$between',
+];
+
+const ARRAY_OPERATORS = ['$in', '$notIn', '$between'];
+
+const OPERATORS = {
+  where: WHERE_OPERATORS,
+  cast: CAST_OPERATORS,
+  group: GROUP_OPERATORS,
+  array: ARRAY_OPERATORS,
+};
+
+// for performance, cache all operators in lowercase
+const OPERATORS_LOWERCASE = Object.fromEntries(
+  Object.entries(OPERATORS).map(([key, values]) => [
+    key,
+    values.map((value) => value.toLowerCase()),
+  ])
+);
+
+const isOperatorOfType = (type, key, ignoreCase = false) => {
+  if (ignoreCase) {
+    return OPERATORS_LOWERCASE[type]?.includes(key.toLowerCase()) ?? false;
+  }
+  return OPERATORS[type]?.includes(key) ?? false;
+};
+
+const isOperator = (key, ignoreCase = false) => {
+  return Object.keys(OPERATORS).some((type) => isOperatorOfType(type, key, ignoreCase));
+};
+
+module.exports = {
+  isOperator,
+  isOperatorOfType,
+  OPERATORS,
+};

packages/core/utils/lib/sanitize/sanitizers.js

@@ -19,6 +19,7 @@ const {
   removeDynamicZones,
   removeMorphToRelations,
 } = require('./visitors');
+const { isOperator } = require('../operators');
 
 const sanitizePasswords = (schema) => async (entity) => {
   return traverseEntity(removePassword, { schema }, entity);
@@ -37,6 +38,17 @@ const defaultSanitizeOutput = async (schema, entity) => {
 
 const defaultSanitizeFilters = curry((schema, filters) => {
   return pipeAsync(
+    // Remove keys that are not attributes or valid operators
+    traverseQueryFilters(
+      ({ key, attribute }, { remove }) => {
+        const isAttribute = !!attribute;
+
+        if (!isAttribute && !isOperator(key) && key !== 'id') {
+          remove(key);
+        }
+      },
+      { schema }
+    ),
     // Remove dynamic zones from filters
     traverseQueryFilters(removeDynamicZones, { schema }),
     // Remove morpTo relations from filters

yarn.lock

@@ -8045,6 +8045,7 @@ __metadata:
   version: 0.0.0-use.local
   resolution: "@strapi/database@workspace:packages/core/database"
   dependencies:
+    "@strapi/utils": 4.10.7
     date-fns: 2.30.0
     debug: 4.3.4
     fs-extra: 10.0.0