Merge pull request #20658 from mozilla/FXA-13808

vpomerleau · web-flow · commit d13ec42f23a4 · 2026-06-17T08:51:28.000-07:00
fix(passkeys): validate base64url WebAuthn fields at the route boundary
diff --git a/packages/fxa-auth-server/lib/routes/passkeys.spec.ts b/packages/fxa-auth-server/lib/routes/passkeys.spec.ts
@@ -2,6 +2,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+import type { Schema } from 'joi';
 import { Container } from 'typedi';
 import { PasskeyService } from '@fxa/accounts/passkey';
 import { AppError } from '@fxa/accounts/errors';
@@ -1609,4 +1610,295 @@ describe('passkeys routes', () => {
       ).rejects.toThrow('DB unavailable');
     });
   });
+
+  describe('credentialId payload validation', () => {
+    const VALID_CRED_ID = 'A_z-09Aa';
+    const VALID_CHALLENGE = 'A_z-09';
+    const VALID_AUTH_INNER = {
+      clientDataJSON: 'eyJ0eXBlIjoid2ViYXV0aG4uZ2V0In0',
+      authenticatorData: 'SZYN5YgOjGh0NBcPZHZgW4_krrmihjLHmVzzuoMdl2MFAAAAAQ',
+      signature: 'MEUCIQCx',
+    };
+    const VALID_REG_INNER = {
+      clientDataJSON: 'eyJ0eXBlIjoid2ViYXV0aG4uY3JlYXRlIn0',
+      attestationObject: 'o2NmbXRkbm9uZQ',
+    };
+
+    function getSchema(
+      path: string,
+      method: string,
+      kind: 'payload' | 'params'
+    ): Schema {
+      const all = passkeyRoutes(customs, db, config, statsd, glean, log);
+      const route = all.find(
+        (r: any) => r.path === path && r.method === method
+      );
+      if (!route) {
+        throw new Error(`Route not found: ${method} ${path}`);
+      }
+      return route.options.validate[kind];
+    }
+
+    const authPayload = (
+      responseOverride: Record<string, unknown> = {},
+      innerOverride: Record<string, unknown> = {},
+      challenge: string = VALID_CHALLENGE
+    ) => ({
+      response: {
+        id: VALID_CRED_ID,
+        type: 'public-key',
+        response: { ...VALID_AUTH_INNER, ...innerOverride },
+        ...responseOverride,
+      },
+      challenge,
+    });
+
+    const regPayload = (
+      responseOverride: Record<string, unknown> = {},
+      innerOverride: Record<string, unknown> = {},
+      challenge: string = VALID_CHALLENGE
+    ) => ({
+      response: {
+        id: VALID_CRED_ID,
+        type: 'public-key',
+        response: { ...VALID_REG_INNER, ...innerOverride },
+        ...responseOverride,
+      },
+      challenge,
+    });
+
+    describe('POST /passkey/authentication/finish', () => {
+      let schema: Schema;
+      beforeEach(() => {
+        schema = getSchema('/passkey/authentication/finish', 'POST', 'payload');
+      });
+
+      it('accepts a well-formed assertion payload', () => {
+        const { error } = schema.validate(authPayload());
+        expect(error).toBeUndefined();
+      });
+
+      it.each([
+        [
+          'shell-injection probe shape',
+          '(nslookup x.example.com||curl x.example.com)',
+          'string.pattern.base',
+        ],
+        ['contains slash', 'A/B', 'string.pattern.base'],
+        ['contains plus', 'A+B', 'string.pattern.base'],
+        ['contains equals padding', 'AA==', 'string.pattern.base'],
+        ['empty string', '', 'string.empty'],
+      ])('rejects response.id (%s)', (_label, badId, expectedType) => {
+        const { error } = schema.validate(authPayload({ id: badId }));
+        expect(error?.details).toEqual([
+          expect.objectContaining({
+            path: ['response', 'id'],
+            type: expectedType,
+          }),
+        ]);
+      });
+
+      it('rejects response.id that exceeds the max length', () => {
+        const { error } = schema.validate(
+          authPayload({ id: 'A'.repeat(1365) })
+        );
+        expect(error?.details).toEqual([
+          expect.objectContaining({
+            path: ['response', 'id'],
+            type: 'string.max',
+          }),
+        ]);
+      });
+
+      it('rejects a challenge longer than 64 chars', () => {
+        const { error } = schema.validate(authPayload({}, {}, 'A'.repeat(65)));
+        expect(error?.details).toEqual([
+          expect.objectContaining({
+            path: ['challenge'],
+            type: 'string.max',
+          }),
+        ]);
+      });
+
+      it('rejects a non-base64url challenge', () => {
+        const { error } = schema.validate(authPayload({}, {}, 'has/slash'));
+        expect(error?.details).toEqual([
+          expect.objectContaining({
+            path: ['challenge'],
+            type: 'string.pattern.base',
+          }),
+        ]);
+      });
+
+      it.each<[string, Record<string, string>]>([
+        ['clientDataJSON', { clientDataJSON: 'has/slash' }],
+        ['authenticatorData', { authenticatorData: 'has/slash' }],
+        ['signature', { signature: 'has/slash' }],
+        ['userHandle', { userHandle: 'has/slash' }],
+      ])(
+        'rejects a non-base64url response.response.%s',
+        (field: string, innerOverride: Record<string, string>) => {
+          const { error } = schema.validate(authPayload({}, innerOverride));
+          expect(error?.details).toEqual([
+            expect.objectContaining({
+              path: ['response', 'response', field],
+              type: 'string.pattern.base',
+            }),
+          ]);
+        }
+      );
+
+      it.each<[string]>([
+        ['clientDataJSON'],
+        ['authenticatorData'],
+        ['signature'],
+      ])(
+        'rejects when required response.response.%s is missing',
+        (field: string) => {
+          const inner: Record<string, string> = { ...VALID_AUTH_INNER };
+          delete inner[field];
+          const { error } = schema.validate({
+            response: {
+              id: VALID_CRED_ID,
+              type: 'public-key',
+              response: inner,
+            },
+            challenge: VALID_CHALLENGE,
+          });
+          expect(error?.details).toEqual([
+            expect.objectContaining({
+              path: ['response', 'response', field],
+              type: 'any.required',
+            }),
+          ]);
+        }
+      );
+    });
+
+    describe('POST /passkey/registration/finish', () => {
+      let schema: Schema;
+      beforeEach(() => {
+        schema = getSchema('/passkey/registration/finish', 'POST', 'payload');
+      });
+
+      it('accepts a well-formed attestation payload', () => {
+        const { error } = schema.validate(regPayload());
+        expect(error).toBeUndefined();
+      });
+
+      it('rejects a non-base64url response.id', () => {
+        const { error } = schema.validate(regPayload({ id: 'has/slash' }));
+        expect(error?.details).toEqual([
+          expect.objectContaining({
+            path: ['response', 'id'],
+            type: 'string.pattern.base',
+          }),
+        ]);
+      });
+
+      it('rejects a challenge longer than 64 chars', () => {
+        const { error } = schema.validate(regPayload({}, {}, 'A'.repeat(65)));
+        expect(error?.details).toEqual([
+          expect.objectContaining({
+            path: ['challenge'],
+            type: 'string.max',
+          }),
+        ]);
+      });
+
+      it('rejects a non-base64url challenge', () => {
+        const { error } = schema.validate(regPayload({}, {}, 'has/slash'));
+        expect(error?.details).toEqual([
+          expect.objectContaining({
+            path: ['challenge'],
+            type: 'string.pattern.base',
+          }),
+        ]);
+      });
+
+      it.each<[string, Record<string, string>]>([
+        ['clientDataJSON', { clientDataJSON: 'has/slash' }],
+        ['attestationObject', { attestationObject: 'has/slash' }],
+        ['authenticatorData', { authenticatorData: 'has/slash' }],
+        ['publicKey', { publicKey: 'has/slash' }],
+      ])(
+        'rejects a non-base64url response.response.%s',
+        (field: string, innerOverride: Record<string, string>) => {
+          const { error } = schema.validate(regPayload({}, innerOverride));
+          expect(error?.details).toEqual([
+            expect.objectContaining({
+              path: ['response', 'response', field],
+              type: 'string.pattern.base',
+            }),
+          ]);
+        }
+      );
+
+      it.each<[string]>([['clientDataJSON'], ['attestationObject']])(
+        'rejects when required response.response.%s is missing',
+        (field: string) => {
+          const inner: Record<string, string> = { ...VALID_REG_INNER };
+          delete inner[field];
+          const { error } = schema.validate({
+            response: {
+              id: VALID_CRED_ID,
+              type: 'public-key',
+              response: inner,
+            },
+            challenge: VALID_CHALLENGE,
+          });
+          expect(error?.details).toEqual([
+            expect.objectContaining({
+              path: ['response', 'response', field],
+              type: 'any.required',
+            }),
+          ]);
+        }
+      );
+    });
+
+    describe('DELETE /passkey/{credentialId}', () => {
+      let schema: Schema;
+      beforeEach(() => {
+        schema = getSchema('/passkey/{credentialId}', 'DELETE', 'params');
+      });
+
+      it('accepts a base64url credentialId', () => {
+        const { error } = schema.validate({ credentialId: VALID_CRED_ID });
+        expect(error).toBeUndefined();
+      });
+
+      it('rejects a non-base64url credentialId', () => {
+        const { error } = schema.validate({ credentialId: 'has/slash' });
+        expect(error?.details).toEqual([
+          expect.objectContaining({
+            path: ['credentialId'],
+            type: 'string.pattern.base',
+          }),
+        ]);
+      });
+    });
+
+    describe('PATCH /passkey/{credentialId}', () => {
+      let schema: Schema;
+      beforeEach(() => {
+        schema = getSchema('/passkey/{credentialId}', 'PATCH', 'params');
+      });
+
+      it('accepts a base64url credentialId', () => {
+        const { error } = schema.validate({ credentialId: VALID_CRED_ID });
+        expect(error).toBeUndefined();
+      });
+
+      it('rejects a non-base64url credentialId', () => {
+        const { error } = schema.validate({ credentialId: 'has/slash' });
+        expect(error?.details).toEqual([
+          expect.objectContaining({
+            path: ['credentialId'],
+            type: 'string.pattern.base',
+          }),
+        ]);
+      });
+    });
+  });
 });
diff --git a/packages/fxa-auth-server/lib/routes/passkeys.ts b/packages/fxa-auth-server/lib/routes/passkeys.ts
@@ -27,6 +27,17 @@ import { FxaMailerFormat } from '../senders/fxa-mailer-format';
 import { OAuthClientInfoServiceName } from '../senders/oauth_client_info';
 import { reportSentryError } from '../sentry';
 
+// Without these, malformed base64url values throw inside SimpleWebAuthn /
+// `base64urlToBuffer` and Hapi surfaces a 500. Route-boundary validation
+// turns probes into 400s and keeps them out of Sentry. CredentialId max
+// matches the WebAuthn L2 ceiling (1023 bytes → 1364 base64url chars),
+// aligning with `passkeys.credentialId VARBINARY(1023)`.
+const BASE64URL_PATTERN = /^[A-Za-z0-9_-]+$/;
+const base64urlString = (maxLen: number) =>
+  isA.string().max(maxLen).regex(BASE64URL_PATTERN);
+const base64urlCredentialId = () => base64urlString(1364);
+const base64urlChallenge = () => base64urlString(64);
+
 /** Subset of the Customs service used by passkey routes. */
 interface Customs {
   /**
@@ -783,8 +794,24 @@ export const passkeyRoutes = (
         },
         validate: {
           payload: isA.object({
-            response: isA.object().required(),
-            challenge: isA.string().required(),
+            response: isA
+              .object({
+                id: base64urlCredentialId().required(),
+                rawId: base64urlCredentialId().optional(),
+                type: isA.string().valid('public-key').required(),
+                response: isA
+                  .object({
+                    clientDataJSON: base64urlString(2048).required(),
+                    attestationObject: base64urlString(32768).required(),
+                    authenticatorData: base64urlString(16384).optional(),
+                    publicKey: base64urlString(1024).optional(),
+                  })
+                  .unknown(true)
+                  .required(),
+              })
+              .unknown(true)
+              .required(),
+            challenge: base64urlChallenge().required(),
           }),
         },
         response: {
@@ -851,7 +878,7 @@ export const passkeyRoutes = (
         },
         validate: {
           params: isA.object({
-            credentialId: isA.string().required(),
+            credentialId: base64urlCredentialId().required(),
           }),
         },
         response: {
@@ -903,16 +930,22 @@ export const passkeyRoutes = (
           payload: isA.object({
             response: isA
               .object({
-                id: isA.string().required(),
+                id: base64urlCredentialId().required(),
+                rawId: base64urlCredentialId().optional(),
                 type: isA.string().valid('public-key').required(),
+                response: isA
+                  .object({
+                    clientDataJSON: base64urlString(2048).required(),
+                    authenticatorData: base64urlString(16384).required(),
+                    signature: base64urlString(1024).required(),
+                    userHandle: base64urlString(128).optional(),
+                  })
+                  .unknown(true)
+                  .required(),
               })
               .unknown(true)
               .required(),
-            challenge: isA
-              .string()
-              .max(64)
-              .regex(/^[A-Za-z0-9_-]+$/)
-              .required(),
+            challenge: base64urlChallenge().required(),
             service: validators.service.optional(),
             metricsContext: METRICS_CONTEXT_SCHEMA,
           }),
@@ -945,7 +978,7 @@ export const passkeyRoutes = (
         },
         validate: {
           params: isA.object({
-            credentialId: isA.string().required(),
+            credentialId: base64urlCredentialId().required(),
           }),
           payload: isA.object({
             name: isA.string().min(1).max(255).required(),
