From b8b69f825a113db8282dec021a80e45a9990757d Mon Sep 17 00:00:00 2001 From: Johann150 Date: Sat, 30 Mar 2024 15:46:42 +0100 Subject: [PATCH] activitypub: strict id check TBH I'm still not quite convinced that this is really necessary but also since treating an id mismatch like a redirect, I also don't think it should break anything. --- .../backend/src/remote/activitypub/request.ts | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/packages/backend/src/remote/activitypub/request.ts b/packages/backend/src/remote/activitypub/request.ts index 4904357d3..dae5d62d3 100644 --- a/packages/backend/src/remote/activitypub/request.ts +++ b/packages/backend/src/remote/activitypub/request.ts @@ -4,6 +4,7 @@ import { getUserKeypair } from '@/misc/keypair-store.js'; import { User } from '@/models/entities/user.js'; import { UserKeypair } from '@/models/entities/user-keypair.js'; import { getResponse } from '@/misc/fetch.js'; +import { getApId } from './type.js'; import { createSignedPost, createSignedGet } from './ap-request.js'; import { apRequestChart, federationChart, instanceChart } from '@/services/chart/index.js'; @@ -128,7 +129,25 @@ export async function signedGet(_url: string, user: { id: User['id'] }): Promise if (!isActivitypub(res.headers.get('Content-Type'))) { throw new Error('invalid response content type'); } - return await res.json(); + + const data = await res.json(); + // In theory, activitypub allows for `id` to be null for ephemeral + // objects, but we wouldn't be fetching those with signed get, since + // they are... ephemeral. + const id = new URL(getApId(data)); + if (id.href !== url.href) { + // if the id and fetched url mismatch, treat it as if it was a redirect + // SECURITY: this is to prevent impersonation via improper media files + url = id; + // if this kind of "redirect" happens, there should be at most one more + // redirect since we now have the canonical url. setting to 2 because it + // will be decremented to 1 right away by the for loop. + if (redirects > 2) { + redirects = 2; + } + } else { + return data; + } } }