diff --git a/api-tests/core/strapi/api/sanitize/sanitize-query.test.api.js b/api-tests/core/strapi/api/sanitize/sanitize-query.test.api.js index 3851bae8f0..04b25cbcea 100644 --- a/api-tests/core/strapi/api/sanitize/sanitize-query.test.api.js +++ b/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 } } }); diff --git a/packages/core/database/lib/query/helpers/where.js b/packages/core/database/lib/query/helpers/where.js index 6d2971ad45..795f81217d 100644 --- a/packages/core/database/lib/query/helpers/where.js +++ b/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 (CAST_OPERATORS.includes(operator)) { + if (!isPlainObject(where)) { + 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))); } diff --git a/packages/core/database/package.json b/packages/core/database/package.json index fb4e8df484..e728fba05f 100644 --- a/packages/core/database/package.json +++ b/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", diff --git a/packages/core/utils/lib/convert-query-params.js b/packages/core/utils/lib/convert-query-params.js index efc279535e..013f0d24a7 100644 --- a/packages/core/utils/lib/convert-query-params.js +++ b/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)); diff --git a/packages/core/utils/lib/index.js b/packages/core/utils/lib/index.js index 937944b811..65b848d46f 100644 --- a/packages/core/utils/lib/index.js +++ b/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, }; diff --git a/packages/core/utils/lib/operators.js b/packages/core/utils/lib/operators.js new file mode 100644 index 0000000000..bdb57b866d --- /dev/null +++ b/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, +}; diff --git a/packages/core/utils/lib/sanitize/sanitizers.js b/packages/core/utils/lib/sanitize/sanitizers.js index 67ab23f380..6a1ea49576 100644 --- a/packages/core/utils/lib/sanitize/sanitizers.js +++ b/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 diff --git a/yarn.lock b/yarn.lock index b3df33c580..1d1d96af29 100644 --- a/yarn.lock +++ b/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