Unverified Commit 14a2f8c6 authored by Hong Minhee's avatar Hong Minhee
Browse files

Fix critical authentication bypass vulnerability in inbox handler 

This fixes a severe security vulnerability where activities were processed
before verifying that the HTTP signature key belonged to the claimed actor,
allowing attackers to impersonate any ActivityPub user.

The fix moves the authentication check (doesActorOwnKey) to occur before
calling routeActivity(), ensuring that malicious activities with mismatched
signatures are rejected before any processing occurs.

A comprehensive test case has been added to verify the fix and prevent
regression of this critical security issue.

https://github.com/fedify-dev/fedify/security/advisories/GHSA-6jcc-xgcr-q3h4
parent 78a10fcc

CHANGES.md

+9 −0
Original line number Diff line number Diff line
@@ -8,6 +8,15 @@ Version 1.3.20 


To be released.



 -  Fixed a critical authentication bypass vulnerability in the inbox handler

    that allowed unauthenticated attackers to impersonate any ActivityPub actor.

    The vulnerability occurred because activities were processed before

    verifying that the HTTP Signatures key belonged to the claimed actor.

    Now authentication verification is performed before activity processing to

    prevent actor impersonation attacks.  [[CVE-2025-54888]]



[CVE-2025-54888]: https://github.com/fedify-dev/fedify/security/advisories/GHSA-6jcc-xgcr-q3h4





Version 1.3.19

--------------

src/federation/handler.test.ts

+81 −0
Original line number Diff line number Diff line
@@ -35,6 +35,7 @@ import { 
  respondWithObject,

  respondWithObjectIfAcceptable,

} from "./handler.ts";

import { InboxListenerSet } from "./inbox.ts";

import { MemoryKvStore } from "./kv.ts";



test("acceptsJsonLd()", () => {
@@ -1279,6 +1280,86 @@ test("respondWithObject()", async () => { 
  });

});



test("handleInbox() - authentication bypass vulnerability", async () => {

  // This test reproduces the authentication bypass vulnerability where

  // activities are processed before verifying the signing key belongs

  // to the claimed actor



  let processedActivity: Create | undefined;

  const inboxListeners = new InboxListenerSet<void>();

  inboxListeners.add(Create, (_ctx, activity) => {

    // Track that the malicious activity was processed

    processedActivity = activity;

  });



  // Create malicious activity claiming to be from victim actor

  const maliciousActivity = new Create({

    id: new URL("https://attacker.example.com/activities/malicious"),

    actor: new URL("https://victim.example.com/users/alice"), // Impersonating victim

    object: new Note({

      id: new URL("https://attacker.example.com/notes/forged"),

      attribution: new URL("https://victim.example.com/users/alice"),

      content: "This is a forged message from the victim!",

    }),

  });



  // Sign request with attacker's key (not victim's key)

  const maliciousRequest = await signRequest(

    new Request("https://example.com/", {

      method: "POST",

      body: JSON.stringify(await maliciousActivity.toJsonLd()),

    }),

    rsaPrivateKey3, // Attacker's private key

    rsaPublicKey3.id!, // Attacker's public key ID

  );



  const maliciousContext = createRequestContext({

    request: maliciousRequest,

    url: new URL(maliciousRequest.url),

    data: undefined,

    documentLoader: mockDocumentLoader,

  });



  const actorDispatcher: ActorDispatcher<void> = (_ctx, identifier) => {

    if (identifier !== "someone") return null;

    return new Person({ name: "Someone" });

  };



  const response = await handleInbox(maliciousRequest, {

    recipient: "someone",

    context: maliciousContext,

    inboxContextFactory(_activity) {

      return createInboxContext({ ...maliciousContext, recipient: "someone" });

    },

    kv: new MemoryKvStore(),

    kvPrefixes: {

      activityIdempotence: ["_fedify", "activityIdempotence"],

      publicKey: ["_fedify", "publicKey"],

    },

    actorDispatcher,

    inboxListeners,

    onNotFound: () => new Response("Not found", { status: 404 }),

    signatureTimeWindow: { minutes: 5 },

    skipSignatureVerification: false,

  });



  // The vulnerability: Even though the response is 401 (unauthorized),

  // the malicious activity was already processed by routeActivity()

  assertEquals(response.status, 401);

  assertEquals(await response.text(), "The signer and the actor do not match.");



  assertEquals(

    processedActivity,

    undefined,

    `SECURITY VULNERABILITY: Malicious activity with mismatched signature was processed! ` +

      `Activity ID: ${processedActivity?.id?.href}, ` +

      `Claimed actor: ${processedActivity?.actorId?.href}`,

  );



  // If we reach here, the vulnerability is fixed - activities with mismatched

  // signatures are properly rejected before processing

});



test("respondWithObjectIfAcceptable", async () => {

  let request = new Request("https://example.com/", {

    headers: { Accept: "application/activity+json" },

src/federation/handler.ts

+14 −14
Original line number Diff line number Diff line
@@ -649,20 +649,6 @@ async function handleInboxInternal<TContextData>( 
    span.setAttribute("activitypub.activity.id", activity.id.href);

  }

  span.setAttribute("activitypub.activity.type", getTypeId(activity).href);

  const routeResult = await routeActivity({

    context: ctx,

    json,

    activity,

    recipient,

    inboxListeners,

    inboxContextFactory,

    inboxErrorHandler,

    kv,

    kvPrefixes,

    queue,

    span,

    tracerProvider,

  });

  if (

    httpSigKey != null && !await doesActorOwnKey(activity, httpSigKey, ctx)

  ) {
@@ -685,6 +671,20 @@ async function handleInboxInternal<TContextData>( 
      headers: { "Content-Type": "text/plain; charset=utf-8" },

    });

  }

  const routeResult = await routeActivity({

    context: ctx,

    json,

    activity,

    recipient,

    inboxListeners,

    inboxContextFactory,

    inboxErrorHandler,

    kv,

    kvPrefixes,

    queue,

    span,

    tracerProvider,

  });

  if (routeResult === "alreadyProcessed") {

    return new Response(

      `Activity <${activity.id}> has already been processed.`,