Skip to content

Commit e474205

Browse files
authored
fix: stricter input validation (#15868)
Misc input validation improvements, sanitizing path segments in both SQL and JSON queries, standardizing the processing of column and JSON paths across different adapters, and making adjustments to traversal and alias generation to align behavior across components.
1 parent 94d2249 commit e474205

File tree

52 files changed

+1805
-135
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

52 files changed

+1805
-135
lines changed

packages/drizzle/src/find/traverseFields.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import { getArrayRelationName } from '../utilities/getArrayRelationName.js'
2727
import { getNameFromDrizzleTable } from '../utilities/getNameFromDrizzleTable.js'
2828
import { jsonAggBuildObject } from '../utilities/json.js'
2929
import { rawConstraint } from '../utilities/rawConstraint.js'
30+
import { sanitizePathSegment } from '../utilities/sanitizePathSegment.js'
3031
import {
3132
InternalBlockTableNameIndex,
3233
resolveBlockTableName,
@@ -63,6 +64,9 @@ const buildSQLWhere = (where: Where, alias: string) => {
6364

6465
const value = where[k][payloadOperator]
6566
if (payloadOperator === '$raw') {
67+
if (typeof value !== 'string') {
68+
return undefined
69+
}
6670
return sql.raw(value)
6771
}
6872

@@ -75,7 +79,16 @@ const buildSQLWhere = (where: Where, alias: string) => {
7579
payloadOperator = 'isNull'
7680
}
7781

78-
return operatorMap[payloadOperator](sql.raw(`"${alias}"."${k.split('.').join('_')}"`), value)
82+
if (!(payloadOperator in operatorMap)) {
83+
return undefined
84+
}
85+
86+
const sanitizedColumnName = k
87+
.split('.')
88+
.map((s) => sanitizePathSegment(s))
89+
.join('_')
90+
91+
return operatorMap[payloadOperator](sql.raw(`"${alias}"."${sanitizedColumnName}"`), value)
7992
}
8093
}
8194
}

packages/drizzle/src/postgres/createJSONQuery/index.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { APIError } from 'payload'
33
import type { CreateJSONQueryArgs } from '../../types.js'
44

55
import { SAFE_STRING_REGEX } from '../../utilities/escapeSQLValue.js'
6+
import { sanitizePathSegment } from '../../utilities/sanitizePathSegment.js'
67

78
const operatorMap: Record<string, string> = {
89
contains: '~',
@@ -43,7 +44,7 @@ export const createJSONQuery = ({ column, operator, pathSegments, value }: Creat
4344
const jsonPaths = pathSegments
4445
.slice(1)
4546
.map((key) => {
46-
return `${key}[*]`
47+
return `${sanitizePathSegment(key)}[*]`
4748
})
4849
.join('.')
4950

packages/drizzle/src/queries/parseParams.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,11 @@ export function parseParams({
185185
}
186186

187187
let formattedValue = val
188-
if (adapter.name === 'sqlite' && operator === 'equals' && !isNaN(val)) {
188+
if (
189+
adapter.name === 'sqlite' &&
190+
operator === 'equals' &&
191+
(typeof val === 'number' || typeof val === 'boolean')
192+
) {
189193
formattedValue = val
190194
} else if (['in', 'not_in'].includes(operator) && Array.isArray(val)) {
191195
formattedValue = `(${val.map((v) => `${escapeSQLValue(v)}`).join(',')})`
Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,13 @@
1+
import { sanitizePathSegment } from '../../utilities/sanitizePathSegment.js'
2+
13
export const convertPathToJSONTraversal = (incomingSegments: string[]): string => {
24
const segments = [...incomingSegments]
35
segments.shift()
46

57
return segments.reduce((res, segment) => {
6-
const formattedSegment = Number.isNaN(parseInt(segment)) ? `'${segment}'` : segment
8+
const formattedSegment = Number.isNaN(parseInt(segment))
9+
? `'${sanitizePathSegment(segment)}'`
10+
: segment
711
return `${res}->>${formattedSegment}`
812
}, '')
913
}

packages/drizzle/src/sqlite/createJSONQuery/index.ts

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type { CreateJSONQueryArgs } from '../../types.js'
22

33
import { escapeSQLValue } from '../../utilities/escapeSQLValue.js'
4+
import { sanitizePathSegment } from '../../utilities/sanitizePathSegment.js'
45

56
type FromArrayArgs = {
67
isRoot?: true
@@ -20,11 +21,12 @@ const fromArray = ({
2021
value,
2122
}: FromArrayArgs) => {
2223
const newPathSegments = pathSegments.slice(1)
23-
const alias = `${pathSegments[isRoot ? 0 : 1]}_alias_${newPathSegments.length}`
24+
const sanitizedSegment = sanitizePathSegment(pathSegments[isRoot ? 0 : 1])
25+
const alias = `${sanitizedSegment}_alias_${newPathSegments.length}`
2426

2527
return `EXISTS (
2628
SELECT 1
27-
FROM json_each(${table}.${pathSegments[0]}) AS ${alias}
29+
FROM json_each(${table}.${sanitizePathSegment(pathSegments[0])}) AS ${alias}
2830
WHERE ${createJSONQuery({
2931
operator,
3032
pathSegments: newPathSegments,
@@ -61,25 +63,25 @@ const createConstraint = ({
6163

6264
if (operator === 'exists') {
6365
if (pathSegments.length === 1) {
64-
return `EXISTS (SELECT 1 FROM json_each("${pathSegments[0]}") AS ${newAlias})`
66+
return `EXISTS (SELECT 1 FROM json_each("${sanitizePathSegment(pathSegments[0])}") AS ${newAlias})`
6567
}
6668

6769
return `EXISTS (
6870
SELECT 1
69-
FROM json_each(${alias}.value -> '${pathSegments[0]}') AS ${newAlias}
70-
WHERE ${newAlias}.key = '${pathSegments[1]}'
71+
FROM json_each(${alias}.value -> '${sanitizePathSegment(pathSegments[0])}') AS ${newAlias}
72+
WHERE ${newAlias}.key = '${sanitizePathSegment(pathSegments[1])}'
7173
)`
7274
}
7375

7476
if (operator === 'not_exists') {
7577
if (pathSegments.length === 1) {
76-
return `NOT EXISTS (SELECT 1 FROM json_each("${pathSegments[0]}") AS ${newAlias})`
78+
return `NOT EXISTS (SELECT 1 FROM json_each("${sanitizePathSegment(pathSegments[0])}") AS ${newAlias})`
7779
}
7880

7981
return `NOT EXISTS (
8082
SELECT 1
81-
FROM json_each(${alias}.value -> '${pathSegments[0]}') AS ${newAlias}
82-
WHERE ${newAlias}.key = '${pathSegments[1]}'
83+
FROM json_each(${alias}.value -> '${sanitizePathSegment(pathSegments[0])}') AS ${newAlias}
84+
WHERE ${newAlias}.key = '${sanitizePathSegment(pathSegments[1])}'
8385
)`
8486
}
8587

@@ -96,13 +98,13 @@ const createConstraint = ({
9698
}
9799

98100
if (pathSegments.length === 1) {
99-
return `EXISTS (SELECT 1 FROM json_each("${pathSegments[0]}") AS ${newAlias} WHERE ${newAlias}.value ${formattedOperator} '${formattedValue}')`
101+
return `EXISTS (SELECT 1 FROM json_each("${sanitizePathSegment(pathSegments[0])}") AS ${newAlias} WHERE ${newAlias}.value ${formattedOperator} '${formattedValue}')`
100102
}
101103

102104
return `EXISTS (
103105
SELECT 1
104-
FROM json_each(${alias}.value -> '${pathSegments[0]}') AS ${newAlias}
105-
WHERE COALESCE(${newAlias}.value ->> '${pathSegments[1]}', '') ${formattedOperator} '${formattedValue}'
106+
FROM json_each(${alias}.value -> '${sanitizePathSegment(pathSegments[0])}') AS ${newAlias}
107+
WHERE COALESCE(${newAlias}.value ->> '${sanitizePathSegment(pathSegments[1])}', '') ${formattedOperator} '${formattedValue}'
106108
)`
107109
}
108110

packages/drizzle/src/utilities/json.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
import type { Column, SQL } from 'drizzle-orm'
22

33
import { sql } from 'drizzle-orm'
4+
import { APIError } from 'payload'
45

56
import type { DrizzleAdapter } from '../types.js'
67

8+
const SAFE_KEY_REGEX = /^\w+$/
9+
710
export function jsonAgg(adapter: DrizzleAdapter, expression: SQL) {
811
if (adapter.name === 'sqlite') {
912
return sql`coalesce(json_group_array(${expression}), '[]')`
@@ -13,7 +16,7 @@ export function jsonAgg(adapter: DrizzleAdapter, expression: SQL) {
1316
}
1417

1518
/**
16-
* @param shape Potential for SQL injections, so you shouldn't allow user-specified key names
19+
* @param shape Keys are interpolated into raw SQL — only use trusted, internally-generated key names
1720
*/
1821
export function jsonBuildObject<T extends Record<string, Column | SQL>>(
1922
adapter: DrizzleAdapter,
@@ -22,6 +25,12 @@ export function jsonBuildObject<T extends Record<string, Column | SQL>>(
2225
const chunks: SQL[] = []
2326

2427
Object.entries(shape).forEach(([key, value]) => {
28+
if (!SAFE_KEY_REGEX.test(key)) {
29+
throw new APIError(
30+
'Unsafe key passed to jsonBuildObject. Only alphanumeric characters and underscores are allowed.',
31+
500,
32+
)
33+
}
2534
if (chunks.length > 0) {
2635
chunks.push(sql.raw(','))
2736
}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import { APIError } from 'payload'
2+
3+
/**
4+
* Validates that a path segment contains only allowed characters (word characters: [a-zA-Z0-9_]).
5+
*
6+
* @throws {APIError} if the segment contains characters outside /^[\w]+$/
7+
*/
8+
const SAFE_PATH_SEGMENT_REGEX = /^\w+$/
9+
10+
export const sanitizePathSegment = (segment: string): string => {
11+
if (!SAFE_PATH_SEGMENT_REGEX.test(segment)) {
12+
throw new APIError(
13+
'Invalid path segment. Only alphanumeric characters and underscores are permitted.',
14+
400,
15+
)
16+
}
17+
return segment
18+
}

packages/next/src/views/Account/ResetPreferences/index.tsx

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,8 @@ export const ResetPreferences: React.FC<{
3535
{
3636
depth: 0,
3737
where: {
38-
user: {
39-
id: {
40-
equals: user.id,
41-
},
38+
'user.value': {
39+
equals: user.id,
4240
},
4341
},
4442
},

packages/payload/src/database/getLocalizedPaths.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
type FlattenedField,
88
} from '../fields/config/types.js'
99
import { APIError, type Payload, type SanitizedCollectionConfig } from '../index.js'
10+
import { SAFE_FIELD_PATH_REGEX } from '../types/constants.js'
1011

1112
export function getLocalizedPaths({
1213
collectionSlug,
@@ -209,7 +210,7 @@ export function getLocalizedPaths({
209210
case 'richText': {
210211
const upcomingSegments = pathSegments.slice(i + 1).join('.')
211212
pathSegments.forEach((path) => {
212-
if (!/^\w+(?:\.\w+)*$/.test(path)) {
213+
if (!SAFE_FIELD_PATH_REGEX.test(path)) {
213214
lastIncompletePath.invalid = true
214215
}
215216
})

packages/payload/src/database/queryValidation/validateQueryPaths.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ export async function validateQueryPaths({
101101
versionFields,
102102
}),
103103
)
104-
} else if (typeof val !== 'object' || val === null) {
104+
} else {
105105
errors.push({ path: `${path}.${operator}` })
106106
}
107107
}

0 commit comments

Comments
 (0)