diff --git a/.changeset/jwt-fail-closed-exp-nbf.md b/.changeset/jwt-fail-closed-exp-nbf.md new file mode 100644 index 00000000000..3af70096f68 --- /dev/null +++ b/.changeset/jwt-fail-closed-exp-nbf.md @@ -0,0 +1,5 @@ +--- +"thirdweb": patch +--- + +Reject JWTs whose nbf or exp is missing or not a finite number instead of skipping the time-bound checks. diff --git a/packages/thirdweb/src/auth/core/verify-jwt.test.ts b/packages/thirdweb/src/auth/core/verify-jwt.test.ts index 4c1f2cd7adb..1a41334a8e8 100644 --- a/packages/thirdweb/src/auth/core/verify-jwt.test.ts +++ b/packages/thirdweb/src/auth/core/verify-jwt.test.ts @@ -3,6 +3,11 @@ import { TEST_ACCOUNT_A, TEST_ACCOUNT_B, } from "../../../test/src/test-wallets.js"; +import { stringToBytes } from "../../utils/encoding/to-bytes.js"; +import { stringify } from "../../utils/json.js"; +import { PRECOMPILED_B64_ENCODED_JWT_HEADER } from "../../utils/jwt/jwt-header.js"; +import { decodeJWT } from "../../utils/jwt/decode-jwt.js"; +import { uint8ArrayToBase64 } from "../../utils/uint8-array.js"; import { generateJWT } from "./generate-jwt.js"; import { generateLoginPayload } from "./generate-login-payload.js"; import { signLoginPayload } from "./sign-login-payload.js"; @@ -10,6 +15,18 @@ import type { AuthOptions } from "./types.js"; import { verifyJWT } from "./verify-jwt.js"; import { verifyLoginPayload } from "./verify-login-payload.js"; +async function signJwtPayload(payload: Record) { + const message = stringify(payload); + const signature = await TEST_ACCOUNT_A.signMessage({ message }); + const encodedData = uint8ArrayToBase64(stringToBytes(message), { + urlSafe: true, + }); + const encodedSignature = uint8ArrayToBase64(stringToBytes(signature), { + urlSafe: true, + }); + return `${PRECOMPILED_B64_ENCODED_JWT_HEADER}.${encodedData}.${encodedSignature}`; +} + const options: AuthOptions = { adminAccount: TEST_ACCOUNT_A, domain: "example.com", @@ -117,4 +134,58 @@ describe("verifyJWT", () => { ); } }); + + test("should fail closed on a non-numeric expiration", async () => { + const jwt = await validJwt(); + const { payload } = decodeJWT(jwt); + const forged = await signJwtPayload({ ...payload, exp: "never" }); + const result = await verifyJWT(options)({ jwt: forged }); + expect(result.valid).toBe(false); + if (!result.valid) { + expect(result.error).toBe("Payload has invalid Expiration Time"); + } + }); + + test("should fail closed when expiration is missing", async () => { + const jwt = await validJwt(); + const { payload } = decodeJWT(jwt); + const { exp: _exp, ...rest } = payload; + const forged = await signJwtPayload(rest); + const result = await verifyJWT(options)({ jwt: forged }); + expect(result.valid).toBe(false); + if (!result.valid) { + expect(result.error).toBe("Payload has invalid Expiration Time"); + } + }); + + test("should fail closed on a non-numeric not-before", async () => { + const jwt = await validJwt(); + const { payload } = decodeJWT(jwt); + const forged = await signJwtPayload({ ...payload, nbf: null }); + const result = await verifyJWT(options)({ jwt: forged }); + expect(result.valid).toBe(false); + if (!result.valid) { + expect(result.error).toBe("Payload has invalid Not Before time"); + } + }); }); + +async function validJwt() { + const generatedPayload = await generateLoginPayload(options)({ + address: TEST_ACCOUNT_B.address, + }); + const signedPayload = await signLoginPayload({ + account: TEST_ACCOUNT_B, + payload: generatedPayload, + }); + const verifiedPayload = await verifyLoginPayload(options)({ + payload: signedPayload.payload, + signature: signedPayload.signature, + }); + if (!verifiedPayload.valid) { + throw new Error("Invalid payload"); + } + return generateJWT(options)({ + payload: verifiedPayload.payload, + }); +} diff --git a/packages/thirdweb/src/auth/core/verify-jwt.ts b/packages/thirdweb/src/auth/core/verify-jwt.ts index b6d7c165007..c85398d5284 100644 --- a/packages/thirdweb/src/auth/core/verify-jwt.ts +++ b/packages/thirdweb/src/auth/core/verify-jwt.ts @@ -57,19 +57,36 @@ export function verifyJWT(options: AuthOptions) { }; } + // Invalid Date / missing NumericDate comparisons are always false in JS, + // so a non-finite nbf or exp previously skipped the time bounds. + const nbf = finiteEpoch(payload.nbf); + if (nbf === undefined) { + return { + error: "Payload has invalid Not Before time", + valid: false, + }; + } + const exp = finiteEpoch(payload.exp); + if (exp === undefined) { + return { + error: "Payload has invalid Expiration Time", + valid: false, + }; + } + // Check that the token is past the invalid before time const currentTime = Math.floor(Date.now() / 1000); - if (currentTime < payload.nbf) { + if (currentTime < nbf) { return { - error: `This token is invalid before epoch time '${payload.nbf}', current epoch time is '${currentTime}'`, + error: `This token is invalid before epoch time '${nbf}', current epoch time is '${currentTime}'`, valid: false, }; } // Check that the token hasn't expired - if (currentTime > payload.exp) { + if (currentTime > exp) { return { - error: `This token expired at epoch time '${payload.exp}', current epoch time is '${currentTime}'`, + error: `This token expired at epoch time '${exp}', current epoch time is '${currentTime}'`, valid: false, }; } @@ -101,3 +118,10 @@ export function verifyJWT(options: AuthOptions) { }; }; } + +function finiteEpoch(value: unknown): number | undefined { + if (typeof value !== "number" || !Number.isFinite(value)) { + return undefined; + } + return value; +}