fix: cap expires in and return bad request

Signed-off-by: ferhat elmas <elmas.ferhat@gmail.com>

Author: ferhatelmas

src/http/routes/object/getSignedURL.ts

@@ -1,3 +1,4 @@
+import { MAX_NUMERIC_JWT_EXPIRATION } from '@internal/auth'
 import { isImageTransformationEnabled } from '@storage/limits'
 import { ImageRenderer } from '@storage/renderer'
 import { FastifyInstance } from 'fastify'
@@ -18,7 +19,12 @@ const getSignedURLParamsSchema = {
 const getSignedURLBodySchema = {
   type: 'object',
   properties: {
-    expiresIn: { type: 'integer', minimum: 1, examples: [60000] },
+    expiresIn: {
+      type: 'integer',
+      minimum: 1,
+      maximum: MAX_NUMERIC_JWT_EXPIRATION,
+      examples: [60000],
+    },
     transform: {
       type: 'object',
       properties: transformationOptionsSchema,

src/http/routes/object/getSignedURLs.ts

@@ -1,3 +1,4 @@
+import { MAX_NUMERIC_JWT_EXPIRATION } from '@internal/auth'
 import { FastifyInstance, FastifyRequest } from 'fastify'
 import { FromSchema } from 'json-schema-to-ts'
 import { createDefaultSchema } from '../../routes-helper'
@@ -14,7 +15,12 @@ const getSignedURLsParamsSchema = {
 const getSignedURLsBodySchema = {
   type: 'object',
   properties: {
-    expiresIn: { type: 'integer', minimum: 1, examples: [60000] },
+    expiresIn: {
+      type: 'integer',
+      minimum: 1,
+      maximum: MAX_NUMERIC_JWT_EXPIRATION,
+      examples: [60000],
+    },
     paths: {
       type: 'array',
       items: { type: 'string' },

src/internal/auth/jwt.ts

@@ -24,6 +24,7 @@ const JWT_HMAC_ALGOS = ['HS256', 'HS384', 'HS512']
 const JWT_RSA_ALGOS = ['RS256', 'RS384', 'RS512']
 const JWT_ECC_ALGOS = ['ES256', 'ES384', 'ES512']
 const JWT_ED_ALGOS = ['EdDSA']
+export const MAX_NUMERIC_JWT_EXPIRATION = Number.MAX_SAFE_INTEGER
 
 export type SignedToken = {
   url: string
@@ -231,8 +232,12 @@ export async function signJWT(
 ): Promise<string> {
   const signer = new SignJWT(payload).setIssuedAt()
   if (expiresIn) {
-    const expiresInStr = typeof expiresIn === 'string' ? expiresIn : Math.floor(expiresIn) + 's'
-    signer.setExpirationTime(expiresInStr)
+    const expiresInStr = getJWTExpirationTime(expiresIn)
+    try {
+      signer.setExpirationTime(expiresInStr)
+    } catch (e) {
+      throw ERRORS.InvalidParameter('expiresIn', { error: e as Error })
+    }
   }
 
   if (typeof secret === 'string') {
@@ -246,6 +251,24 @@ export async function signJWT(
   }
 }
 
+function getJWTExpirationTime(expiresIn: string | number) {
+  if (typeof expiresIn === 'string') {
+    return expiresIn
+  }
+
+  if (!Number.isFinite(expiresIn)) {
+    throw ERRORS.InvalidParameter('expiresIn')
+  }
+
+  const expiresInSeconds = Math.floor(expiresIn)
+
+  if (!Number.isSafeInteger(expiresInSeconds) || expiresInSeconds > MAX_NUMERIC_JWT_EXPIRATION) {
+    throw ERRORS.InvalidParameter('expiresIn')
+  }
+
+  return `${expiresInSeconds}s`
+}
+
 /**
  * Generate a new random HS512 JWK that can be used for signing JWTs
  */

src/test/jwt.test.ts

@@ -1,9 +1,16 @@
 import { JWT_CACHE_NAME } from '@internal/cache'
+import { ErrorCode } from '@internal/errors'
 import { cacheRequestsTotal } from '@internal/monitoring/metrics'
 import * as crypto from 'crypto'
 import { SignJWT } from 'jose'
 import { JwksConfigKey } from '../config'
-import { generateHS512JWK, signJWT, verifyJWT, verifyJWTWithCache } from '../internal/auth'
+import {
+  generateHS512JWK,
+  MAX_NUMERIC_JWT_EXPIRATION,
+  signJWT,
+  verifyJWT,
+  verifyJWTWithCache,
+} from '../internal/auth'
 
 describe('JWT', () => {
   describe('verifyJWT with JWKS', () => {
@@ -148,6 +155,22 @@ describe('JWT', () => {
       )
     })
 
+    test('it should allow the configured maximum numeric expiration', async () => {
+      await expect(
+        signJWT({ sub: 'things' }, hmacPrivateKeyWithoutKid, MAX_NUMERIC_JWT_EXPIRATION)
+      ).resolves.toEqual(expect.any(String))
+    })
+
+    test('it should reject numeric expirations above the configured maximum', async () => {
+      await expect(
+        signJWT({ sub: 'things' }, hmacPrivateKeyWithoutKid, MAX_NUMERIC_JWT_EXPIRATION + 1)
+      ).rejects.toMatchObject({
+        code: ErrorCode.InvalidParameter,
+        httpStatusCode: 400,
+        message: 'Invalid Parameter expiresIn',
+      })
+    })
+
     test('it should reject if jwt is malformed', async () => {
       await expect(verifyJWT('this is not a jwt', 'and this is not a secret')).rejects.toThrow(
         'Invalid Compact JWS'

src/test/object.test.ts

@@ -1830,6 +1830,22 @@ describe('testing generating signed URL', () => {
     })
     expect(response.statusCode).toBe(400)
   })
+
+  test('rejects oversized expiresIn values for signed URLs before jwt signing', async () => {
+    const response = await appInstance.inject({
+      method: 'POST',
+      url: '/object/sign/bucket2/authenticated/cat.jpg',
+      headers: {
+        authorization: `Bearer ${process.env.AUTHENTICATED_KEY}`,
+      },
+      payload: {
+        expiresIn: 1e21,
+      },
+    })
+
+    expect(response.statusCode).toBe(400)
+    expect(JSON.parse(response.body).message).toContain('expiresIn')
+  })
 })
 
 /**
@@ -2207,6 +2223,23 @@ describe('testing generating signed URLs', () => {
     const result = JSON.parse(response.body)
     expect(result[0].error).toBe('Either the object does not exist or you do not have access to it')
   })
+
+  test('rejects oversized expiresIn values for batch signed URLs before jwt signing', async () => {
+    const response = await appInstance.inject({
+      method: 'POST',
+      url: '/object/sign/bucket2',
+      headers: {
+        authorization: `Bearer ${process.env.AUTHENTICATED_KEY}`,
+      },
+      payload: {
+        expiresIn: 1e21,
+        paths: ['authenticated/cat.jpg'],
+      },
+    })
+
+    expect(response.statusCode).toBe(400)
+    expect(JSON.parse(response.body).message).toContain('expiresIn')
+  })
 })
 
 /**