Skip to content

Commit a742140

Browse files
fix: prevent multipart content-type backtracking (#17679)
Updates multipart `Content-Type` validation so parameter separators cannot also be consumed as parameter content. This allows malformed values to be rejected promptly while preserving the multipart subtype and parameter formats Payload already supports. --------- Co-authored-by: Patrik Kozak <35232443+PatrikKozak@users.noreply.github.com>
1 parent 025581d commit a742140

28 files changed

Lines changed: 936 additions & 79 deletions

File tree

‎examples/auth/src/app/(app)/create-account/CreateAccountForm/index.tsx‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import Link from 'next/link'
44
import { useRouter, useSearchParams } from 'next/navigation'
5+
import { getSafeRedirect } from 'payload/shared'
56
import React, { useCallback, useRef, useState } from 'react'
67
import { useForm } from 'react-hook-form'
78

@@ -51,7 +52,10 @@ export const CreateAccountForm: React.FC = () => {
5152
return
5253
}
5354

54-
const redirect = searchParams.get('redirect')
55+
const redirect = getSafeRedirect({
56+
fallbackTo: `/account?success=${encodeURIComponent('Account created successfully')}`,
57+
redirectTo: searchParams.get('redirect') ?? '',
58+
})
5559

5660
const timer = setTimeout(() => {
5761
setLoading(true)
@@ -60,8 +64,7 @@ export const CreateAccountForm: React.FC = () => {
6064
try {
6165
await login(data)
6266
clearTimeout(timer)
63-
if (redirect) {router.push(redirect)}
64-
else {router.push(`/account?success=${encodeURIComponent('Account created successfully')}`)}
67+
router.push(redirect)
6568
} catch (_) {
6669
clearTimeout(timer)
6770
setError('There was an error with the credentials provided. Please try again.')

‎examples/auth/src/app/(app)/login/LoginForm/index.tsx‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import Link from 'next/link'
44
import { useRouter, useSearchParams } from 'next/navigation'
5+
import { getSafeRedirect } from 'payload/shared'
56
import React, { useCallback, useRef } from 'react'
67
import { useForm } from 'react-hook-form'
78

@@ -19,7 +20,12 @@ type FormData = {
1920
export const LoginForm: React.FC = () => {
2021
const searchParams = useSearchParams()
2122
const allParams = searchParams.toString() ? `?${searchParams.toString()}` : ''
22-
const redirect = useRef(searchParams.get('redirect'))
23+
const redirect = useRef(
24+
getSafeRedirect({
25+
fallbackTo: '/account',
26+
redirectTo: searchParams.get('redirect') ?? '',
27+
}),
28+
)
2329
const { login } = useAuth()
2430
const router = useRouter()
2531
const [error, setError] = React.useState<null | string>(null)
@@ -39,8 +45,7 @@ export const LoginForm: React.FC = () => {
3945
async (data: FormData) => {
4046
try {
4147
await login(data)
42-
if (redirect?.current) {router.push(redirect.current)}
43-
else {router.push('/account')}
48+
router.push(redirect.current)
4449
} catch (_) {
4550
setError('There was an error with the credentials provided. Please try again.')
4651
}

‎packages/payload/src/collections/operations/find.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import type {
1818
import { executeAccess } from '../../auth/executeAccess.js'
1919
import { combineQueries } from '../../database/combineQueries.js'
2020
import { validateQueryPaths } from '../../database/queryValidation/validateQueryPaths.js'
21+
import { validateSortQuery } from '../../database/queryValidation/validateSortQuery.js'
2122
import { sanitizeJoinQuery } from '../../database/sanitizeJoinQuery.js'
2223
import { sanitizeWhereQuery } from '../../database/sanitizeWhereQuery.js'
2324
import { afterRead } from '../../fields/hooks/afterRead/index.js'
@@ -158,7 +159,14 @@ export const findOperation = async <
158159

159160
const sort = sanitizeSortQuery({
160161
fields: collection.config.flattenedFields,
161-
sort: incomingSort,
162+
sort: incomingSort || collectionConfig.defaultSort,
163+
})
164+
165+
await validateSortQuery({
166+
collectionConfig,
167+
overrideAccess: overrideAccess!,
168+
req,
169+
sort,
162170
})
163171

164172
const sanitizedJoins = await sanitizeJoinQuery({

‎packages/payload/src/collections/operations/findDistinct.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import type { Collection } from '../config/types.js'
99
import { executeAccess } from '../../auth/executeAccess.js'
1010
import { combineQueries } from '../../database/combineQueries.js'
1111
import { validateQueryPaths } from '../../database/queryValidation/validateQueryPaths.js'
12+
import { validateSortQuery } from '../../database/queryValidation/validateSortQuery.js'
1213
import { sanitizeWhereQuery } from '../../database/sanitizeWhereQuery.js'
1314
import { APIError } from '../../errors/APIError.js'
1415
import { Forbidden } from '../../errors/Forbidden.js'
@@ -137,6 +138,13 @@ export const findDistinctOperation = async (
137138
}
138139
}
139140

141+
await validateSortQuery({
142+
collectionConfig,
143+
overrideAccess: overrideAccess!,
144+
req,
145+
sort: args.sort,
146+
})
147+
140148
if ('virtual' in fieldResult.field && fieldResult.field.virtual) {
141149
if (typeof fieldResult.field.virtual !== 'string') {
142150
throw new APIError(

‎packages/payload/src/collections/operations/findVersions.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import type { FindOptions } from './local/find.js'
88
import { executeAccess } from '../../auth/executeAccess.js'
99
import { combineQueries } from '../../database/combineQueries.js'
1010
import { validateQueryPaths } from '../../database/queryValidation/validateQueryPaths.js'
11+
import { validateSortQuery } from '../../database/queryValidation/validateSortQuery.js'
1112
import { sanitizeWhereQuery } from '../../database/sanitizeWhereQuery.js'
1213
import { afterRead } from '../../fields/hooks/afterRead/index.js'
1314
import { appendNonTrashedFilter } from '../../utilities/appendNonTrashedFilter.js'
@@ -87,6 +88,14 @@ export const findVersionsOperation = async <TData extends TypeWithVersion<TData>
8788
where: where!,
8889
})
8990

91+
await validateSortQuery({
92+
collectionConfig,
93+
overrideAccess: overrideAccess!,
94+
req,
95+
sort,
96+
versionFields,
97+
})
98+
9099
let fullWhere = combineQueries(where!, accessResults)
91100

92101
// Exclude trashed documents when trash: false

‎packages/payload/src/collections/operations/update.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import type {
1515
import { executeAccess } from '../../auth/executeAccess.js'
1616
import { combineQueries } from '../../database/combineQueries.js'
1717
import { validateQueryPaths } from '../../database/queryValidation/validateQueryPaths.js'
18+
import { validateSortQuery } from '../../database/queryValidation/validateSortQuery.js'
1819
import { sanitizeWhereQuery } from '../../database/sanitizeWhereQuery.js'
1920
import { APIError } from '../../errors/index.js'
2021
import { type CollectionSlug, type FindOptions } from '../../index.js'
@@ -175,7 +176,14 @@ export const updateOperation = async <
175176

176177
const sort = sanitizeSortQuery({
177178
fields: collection.config.flattenedFields,
178-
sort: incomingSort,
179+
sort: incomingSort || collectionConfig.defaultSort,
180+
})
181+
182+
await validateSortQuery({
183+
collectionConfig,
184+
overrideAccess: overrideAccess!,
185+
req,
186+
sort,
179187
})
180188

181189
let docs

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ export async function validateQueryPaths({
5050
for (const path in where) {
5151
const constraint = where[path]
5252

53-
if ((path === 'and' || path === 'or') && Array.isArray(constraint)) {
53+
if (['and', 'or'].includes(path.toLowerCase()) && Array.isArray(constraint)) {
5454
for (const item of constraint) {
5555
if (collectionConfig) {
5656
promises.push(
@@ -80,6 +80,8 @@ export async function validateQueryPaths({
8080
)
8181
}
8282
}
83+
} else if (Array.isArray(constraint)) {
84+
errors.push({ path })
8385
} else if (!Array.isArray(constraint)) {
8486
for (const operator in constraint) {
8587
const val = constraint[operator as keyof typeof constraint]
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
import type { SanitizedCollectionConfig } from '../../collections/config/types.js'
2+
import type { FlattenedField } from '../../fields/config/types.js'
3+
import type { SanitizedGlobalConfig } from '../../globals/config/types.js'
4+
import type { PayloadRequest, Sort, Where } from '../../types/index.js'
5+
6+
import { getLocalizedPaths } from '../getLocalizedPaths.js'
7+
import { validateQueryPaths } from './validateQueryPaths.js'
8+
9+
type Args = {
10+
overrideAccess: boolean
11+
req: PayloadRequest
12+
sort?: Sort
13+
versionFields?: FlattenedField[]
14+
} & (
15+
| {
16+
collectionConfig: SanitizedCollectionConfig
17+
globalConfig?: undefined
18+
}
19+
| {
20+
collectionConfig?: undefined
21+
globalConfig: SanitizedGlobalConfig
22+
}
23+
)
24+
25+
/**
26+
* Ensures sort paths are subject to the same field-read access checks as query `where` paths,
27+
* since database sorting happens before response-time field redaction.
28+
*/
29+
export const validateSortQuery = async ({
30+
collectionConfig,
31+
globalConfig,
32+
overrideAccess,
33+
req,
34+
sort,
35+
versionFields,
36+
}: Args): Promise<void> => {
37+
if (overrideAccess || !sort) {
38+
return
39+
}
40+
41+
const fields = versionFields || (globalConfig || collectionConfig).flattenedFields
42+
const sortFields = Array.isArray(sort) ? sort : [sort]
43+
const where: Where = {}
44+
45+
for (const sortField of sortFields) {
46+
const path = sortField.replace(/^-/, '').replace(/__/g, '.')
47+
const paths = getLocalizedPaths({
48+
collectionSlug: collectionConfig?.slug,
49+
fields,
50+
globalSlug: globalConfig?.slug,
51+
incomingPath: path,
52+
locale: req.locale!,
53+
overrideAccess: true,
54+
payload: req.payload,
55+
})
56+
57+
if (path !== 'id' && path !== '_id' && paths.every(({ invalid }) => !invalid)) {
58+
where[path] = { exists: true }
59+
}
60+
}
61+
62+
if (Object.keys(where).length === 0) {
63+
return
64+
}
65+
66+
if (collectionConfig) {
67+
await validateQueryPaths({
68+
collectionConfig,
69+
overrideAccess,
70+
req,
71+
versionFields,
72+
where,
73+
})
74+
} else {
75+
await validateQueryPaths({
76+
globalConfig,
77+
overrideAccess,
78+
req,
79+
versionFields,
80+
where,
81+
})
82+
}
83+
}

‎packages/payload/src/database/sanitizeJoinQuery.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { executeAccess } from '../auth/executeAccess.js'
55
import { QueryError } from '../errors/QueryError.js'
66
import { combineQueries } from './combineQueries.js'
77
import { validateQueryPaths } from './queryValidation/validateQueryPaths.js'
8+
import { validateSortQuery } from './queryValidation/validateSortQuery.js'
89
import { sanitizeWhereQuery } from './sanitizeWhereQuery.js'
910

1011
type Args = {
@@ -74,6 +75,12 @@ const sanitizeJoinFieldQuery = async ({
7475
// incoming where input, but we shouldn't validate generated from the access control.
7576
where: joinQuery.where,
7677
}),
78+
validateSortQuery({
79+
collectionConfig: joinCollectionConfig,
80+
overrideAccess,
81+
req,
82+
sort: joinQuery.sort || join.field.defaultSort || joinCollectionConfig.defaultSort,
83+
}),
7784
)
7885

7986
if (typeof accessResult === 'object') {

‎packages/payload/src/globals/operations/findVersions.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import type { SanitizedGlobalConfig } from '../config/types.js'
77
import { executeAccess } from '../../auth/executeAccess.js'
88
import { combineQueries } from '../../database/combineQueries.js'
99
import { validateQueryPaths } from '../../database/queryValidation/validateQueryPaths.js'
10+
import { validateSortQuery } from '../../database/queryValidation/validateSortQuery.js'
1011
import { afterRead } from '../../fields/hooks/afterRead/index.js'
1112
import { killTransaction } from '../../utilities/killTransaction.js'
1213
import { sanitizeInternalFields } from '../../utilities/sanitizeInternalFields.js'
@@ -66,6 +67,14 @@ export const findVersionsOperation = async <T extends TypeWithVersion<T>>(
6667
where: where!,
6768
})
6869

70+
await validateSortQuery({
71+
globalConfig,
72+
overrideAccess: overrideAccess!,
73+
req,
74+
sort,
75+
versionFields,
76+
})
77+
6978
const fullWhere = combineQueries(where!, accessResults)
7079

7180
const select = sanitizeSelect({

0 commit comments

Comments
 (0)