Skip to content

Commit 106ae6e

Browse files
committed
fix: cap expires in and return bad request
Signed-off-by: ferhat elmas <elmas.ferhat@gmail.com>
1 parent c863126 commit 106ae6e

5 files changed

Lines changed: 177 additions & 9 deletions

File tree

src/http/routes/object/getSignedURL.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { assertValidNumericJWTExpiration } from '@internal/auth'
12
import { isImageTransformationEnabled } from '@storage/limits'
23
import { ImageRenderer } from '@storage/renderer'
34
import { FastifyInstance } from 'fastify'
@@ -18,7 +19,11 @@ const getSignedURLParamsSchema = {
1819
const getSignedURLBodySchema = {
1920
type: 'object',
2021
properties: {
21-
expiresIn: { type: 'integer', minimum: 1, examples: [60000] },
22+
expiresIn: {
23+
type: 'integer',
24+
minimum: 1,
25+
examples: [60000],
26+
},
2227
transform: {
2328
type: 'object',
2429
properties: transformationOptionsSchema,
@@ -66,6 +71,7 @@ export default async function routes(fastify: FastifyInstance) {
6671
const { bucketName } = request.params
6772
const objectName = request.params['*']
6873
const { expiresIn } = request.body
74+
assertValidNumericJWTExpiration(expiresIn)
6975

7076
const urlPath = request.url.split('?').shift()
7177
const imageTransformationEnabled = await isImageTransformationEnabled(request.tenantId)

src/http/routes/object/getSignedURLs.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { assertValidNumericJWTExpiration } from '@internal/auth'
12
import { FastifyInstance, FastifyRequest } from 'fastify'
23
import { FromSchema } from 'json-schema-to-ts'
34
import { createDefaultSchema } from '../../routes-helper'
@@ -14,7 +15,11 @@ const getSignedURLsParamsSchema = {
1415
const getSignedURLsBodySchema = {
1516
type: 'object',
1617
properties: {
17-
expiresIn: { type: 'integer', minimum: 1, examples: [60000] },
18+
expiresIn: {
19+
type: 'integer',
20+
minimum: 1,
21+
examples: [60000],
22+
},
1823
paths: {
1924
type: 'array',
2025
items: { type: 'string' },
@@ -77,6 +82,7 @@ export default async function routes(fastify: FastifyInstance) {
7782
async (request, response) => {
7883
const { bucketName } = request.params
7984
const { expiresIn, paths } = request.body
85+
assertValidNumericJWTExpiration(expiresIn)
8086

8187
const signedURLs = await request.storage.from(bucketName).signObjectUrls(paths, expiresIn)
8288

src/internal/auth/jwt.ts

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ const JWT_HMAC_ALGOS = ['HS256', 'HS384', 'HS512']
2424
const JWT_RSA_ALGOS = ['RS256', 'RS384', 'RS512']
2525
const JWT_ECC_ALGOS = ['ES256', 'ES384', 'ES512']
2626
const JWT_ED_ALGOS = ['EdDSA']
27+
const MAX_ABSOLUTE_JWT_EXPIRATION_SECONDS = Math.floor(Number.MAX_SAFE_INTEGER / 1000)
2728

2829
export type SignedToken = {
2930
url: string
@@ -230,9 +231,13 @@ export async function signJWT(
230231
expiresIn: string | number | undefined
231232
): Promise<string> {
232233
const signer = new SignJWT(payload).setIssuedAt()
233-
if (expiresIn) {
234-
const expiresInStr = typeof expiresIn === 'string' ? expiresIn : Math.floor(expiresIn) + 's'
235-
signer.setExpirationTime(expiresInStr)
234+
if (expiresIn !== undefined) {
235+
const expiresInStr = getJWTExpirationTime(expiresIn)
236+
try {
237+
signer.setExpirationTime(expiresInStr)
238+
} catch (e) {
239+
throw ERRORS.InvalidParameter('expiresIn', { error: e as Error })
240+
}
236241
}
237242

238243
if (typeof secret === 'string') {
@@ -246,6 +251,37 @@ export async function signJWT(
246251
}
247252
}
248253

254+
function getJWTExpirationTime(expiresIn: string | number) {
255+
if (typeof expiresIn === 'string') {
256+
return expiresIn
257+
}
258+
259+
assertValidNumericJWTExpiration(expiresIn)
260+
return `${Math.floor(expiresIn)}s`
261+
}
262+
263+
export function getMaxNumericJWTExpiration(nowMs = Date.now()) {
264+
const nowSeconds = Math.floor(nowMs / 1000)
265+
return Math.max(0, MAX_ABSOLUTE_JWT_EXPIRATION_SECONDS - nowSeconds)
266+
}
267+
268+
export function assertValidNumericJWTExpiration(expiresIn: number, nowMs = Date.now()) {
269+
if (!Number.isFinite(expiresIn)) {
270+
throw ERRORS.InvalidParameter('expiresIn')
271+
}
272+
273+
const expiresInSeconds = Math.floor(expiresIn)
274+
const maxRelativeExpirationSeconds = getMaxNumericJWTExpiration(nowMs)
275+
276+
if (
277+
!Number.isSafeInteger(expiresInSeconds) ||
278+
expiresInSeconds < 1 ||
279+
expiresInSeconds > maxRelativeExpirationSeconds
280+
) {
281+
throw ERRORS.InvalidParameter('expiresIn')
282+
}
283+
}
284+
249285
/**
250286
* Generate a new random HS512 JWK that can be used for signing JWTs
251287
*/

src/test/jwt.test.ts

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,17 @@
11
import { JWT_CACHE_NAME } from '@internal/cache'
2+
import { ErrorCode } from '@internal/errors'
23
import { cacheRequestsTotal } from '@internal/monitoring/metrics'
34
import * as crypto from 'crypto'
45
import { SignJWT } from 'jose'
56
import { JwksConfigKey } from '../config'
6-
import { generateHS512JWK, signJWT, verifyJWT, verifyJWTWithCache } from '../internal/auth'
7+
import {
8+
assertValidNumericJWTExpiration,
9+
generateHS512JWK,
10+
getMaxNumericJWTExpiration,
11+
signJWT,
12+
verifyJWT,
13+
verifyJWTWithCache,
14+
} from '../internal/auth'
715

816
describe('JWT', () => {
917
describe('verifyJWT with JWKS', () => {
@@ -148,6 +156,63 @@ describe('JWT', () => {
148156
)
149157
})
150158

159+
test('it should allow the current maximum numeric expiration and keep exp millisecond-safe', async () => {
160+
jest.useFakeTimers()
161+
jest.setSystemTime(new Date('2026-01-01T00:00:00.000Z'))
162+
163+
const maxNumericExpiration = getMaxNumericJWTExpiration()
164+
const jwt = await signJWT({ sub: 'things' }, hmacPrivateKeyWithoutKid, maxNumericExpiration)
165+
const result = await verifyJWT(jwt, hmacPrivateKeyWithoutKid)
166+
167+
expect(maxNumericExpiration).toBeGreaterThan(0)
168+
expect(Number.isSafeInteger(result.exp)).toBe(true)
169+
expect(Number.isSafeInteger(result.exp! * 1000)).toBe(true)
170+
})
171+
172+
test('it should reject numeric expirations above the current maximum', async () => {
173+
jest.useFakeTimers()
174+
jest.setSystemTime(new Date('2026-01-01T00:00:00.000Z'))
175+
176+
const maxNumericExpiration = getMaxNumericJWTExpiration()
177+
await expect(
178+
signJWT({ sub: 'things' }, hmacPrivateKeyWithoutKid, maxNumericExpiration + 1)
179+
).rejects.toMatchObject({
180+
code: ErrorCode.InvalidParameter,
181+
httpStatusCode: 400,
182+
message: 'Invalid Parameter expiresIn',
183+
})
184+
})
185+
186+
test('it should reject numeric expirations above the current maximum in the shared validator', () => {
187+
jest.useFakeTimers()
188+
jest.setSystemTime(new Date('2026-01-01T00:00:00.000Z'))
189+
190+
expect(() => assertValidNumericJWTExpiration(getMaxNumericJWTExpiration() + 1)).toThrow(
191+
'Invalid Parameter expiresIn'
192+
)
193+
})
194+
195+
test('it should reject numeric expirations below one second', async () => {
196+
await expect(signJWT({ sub: 'things' }, hmacPrivateKeyWithoutKid, 0)).rejects.toMatchObject({
197+
code: ErrorCode.InvalidParameter,
198+
httpStatusCode: 400,
199+
message: 'Invalid Parameter expiresIn',
200+
})
201+
202+
await expect(
203+
signJWT({ sub: 'things' }, hmacPrivateKeyWithoutKid, -1)
204+
).rejects.toMatchObject({
205+
code: ErrorCode.InvalidParameter,
206+
httpStatusCode: 400,
207+
message: 'Invalid Parameter expiresIn',
208+
})
209+
})
210+
211+
test('it should reject numeric expirations below one second in the shared validator', () => {
212+
expect(() => assertValidNumericJWTExpiration(0)).toThrow('Invalid Parameter expiresIn')
213+
expect(() => assertValidNumericJWTExpiration(-1)).toThrow('Invalid Parameter expiresIn')
214+
})
215+
151216
test('it should reject if jwt is malformed', async () => {
152217
await expect(verifyJWT('this is not a jwt', 'and this is not a secret')).rejects.toThrow(
153218
'Invalid Compact JWS'

src/test/object.test.ts

Lines changed: 58 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
'use strict'
22

3-
import { generateHS512JWK, SignedToken, signJWT, verifyJWT } from '@internal/auth'
3+
import {
4+
generateHS512JWK,
5+
getMaxNumericJWTExpiration,
6+
SignedToken,
7+
signJWT,
8+
verifyJWT,
9+
} from '@internal/auth'
410
import { getPostgresConnection, getServiceKeyUser } from '@internal/database'
511
import { ErrorCode, StorageBackendError } from '@internal/errors'
612
import { randomUUID } from 'crypto'
@@ -1830,6 +1836,38 @@ describe('testing generating signed URL', () => {
18301836
})
18311837
expect(response.statusCode).toBe(400)
18321838
})
1839+
1840+
test('rejects oversized expiresIn values for signed URLs before jwt signing', async () => {
1841+
const response = await appInstance.inject({
1842+
method: 'POST',
1843+
url: '/object/sign/bucket2/authenticated/cat.jpg',
1844+
headers: {
1845+
authorization: `Bearer ${process.env.AUTHENTICATED_KEY}`,
1846+
},
1847+
payload: {
1848+
expiresIn: 1e21,
1849+
},
1850+
})
1851+
1852+
expect(response.statusCode).toBe(400)
1853+
expect(JSON.parse(response.body).message).toContain('expiresIn')
1854+
})
1855+
1856+
test('rejects expiresIn values above the current runtime maximum for signed URLs', async () => {
1857+
const response = await appInstance.inject({
1858+
method: 'POST',
1859+
url: '/object/sign/bucket2/authenticated/cat.jpg',
1860+
headers: {
1861+
authorization: `Bearer ${process.env.AUTHENTICATED_KEY}`,
1862+
},
1863+
payload: {
1864+
expiresIn: getMaxNumericJWTExpiration() + 10,
1865+
},
1866+
})
1867+
1868+
expect(response.statusCode).toBe(400)
1869+
expect(JSON.parse(response.body).message).toContain('expiresIn')
1870+
})
18331871
})
18341872

18351873
/**
@@ -2034,7 +2072,7 @@ describe('testing uploading with generated signed upload URL', () => {
20342072
const urlToSign = `${BUCKET_ID}/${OBJECT_NAME}`
20352073
const owner = '317eadce-631a-4429-a0bb-f19a7a517b4a'
20362074

2037-
const jwtToken = await signJWT({ owner, url: urlToSign }, jwtSecret, -1)
2075+
const jwtToken = await signJWT({ owner, url: urlToSign }, jwtSecret, '-1s')
20382076
const response = await appInstance.inject({
20392077
method: 'PUT',
20402078
url: `/object/upload/sign/${urlToSign}?token=${jwtToken}`,
@@ -2207,6 +2245,23 @@ describe('testing generating signed URLs', () => {
22072245
const result = JSON.parse(response.body)
22082246
expect(result[0].error).toBe('Either the object does not exist or you do not have access to it')
22092247
})
2248+
2249+
test('rejects oversized expiresIn values for batch signed URLs before jwt signing', async () => {
2250+
const response = await appInstance.inject({
2251+
method: 'POST',
2252+
url: '/object/sign/bucket2',
2253+
headers: {
2254+
authorization: `Bearer ${process.env.AUTHENTICATED_KEY}`,
2255+
},
2256+
payload: {
2257+
expiresIn: 1e21,
2258+
paths: ['authenticated/cat.jpg'],
2259+
},
2260+
})
2261+
2262+
expect(response.statusCode).toBe(400)
2263+
expect(JSON.parse(response.body).message).toContain('expiresIn')
2264+
})
22102265
})
22112266

22122267
/**
@@ -2300,7 +2355,7 @@ describe('testing retrieving signed URL', () => {
23002355

23012356
test('get object with an expired JWT', async () => {
23022357
const urlToSign = 'bucket2/public/sadcat-upload.png'
2303-
const expiredJWT = await signJWT({ url: urlToSign }, jwtSecret, -1)
2358+
const expiredJWT = await signJWT({ url: urlToSign }, jwtSecret, '-1s')
23042359
const response = await appInstance.inject({
23052360
method: 'GET',
23062361
url: `/object/sign/${urlToSign}?token=${expiredJWT}`,

0 commit comments

Comments
 (0)