[backend] Improve validation of AP activities & objects

This commit addresses disclosed primitives 4-5, 7-9, 12-17 & 21-22 (CVE-2024-51403, CVE-2024-51404, CVE-2024-51405)

packages/backend/src/queue/queues/inbox.ts

@@ -178,13 +178,15 @@ async function process(job: Job<InboxJobData>): Promise<string> {
 	}
 
 	// activity.idがあればホストが署名者のホストであることを確認する
-	if (typeof activity.id === "string") {
+	if (typeof activity.id !== "string") {
+		return 'skip: activity.id is not a string';
+	}
+
 	const signerHost = extractDbHost(authUser.user.uri!);
 	const activityIdHost = extractDbHost(activity.id);
 	if (signerHost !== activityIdHost) {
 		return `skip: signerHost(${signerHost}) !== activity.id host(${activityIdHost}`;
 	}
-	}
 
 	// Update stats
 	registerOrFetchInstanceDoc(authUser.user.host).then((i) => {

packages/backend/src/remote/activitypub/check-fetch.ts

@@ -37,7 +37,7 @@ export async function checkFetch(req: IncomingMessage): Promise<number> {
 		let signature;
 
 		try {
-			signature = httpSignature.parseRequest(req, { headers: ["(request-target)", "host", "date"] });
+			signature = httpSignature.parseRequest(req, { headers: ["(request-target)", "host", "date"], authorizationHeaderName: 'signature' });
 		} catch (e) {
 			return 401;
 		}

packages/backend/src/remote/activitypub/kernel/create/note.ts

@@ -29,6 +29,9 @@ export default async function (
 				return "skip: host in actor.uri !== note.id";
 			}
 		}
+		else {
+			return "skip: note.id is not a string";
+		}
 	}
 
 	const unlock = await getApLock(uri);

packages/backend/src/remote/activitypub/kernel/index.ts

@@ -46,19 +46,8 @@ export async function performActivity(
 	activity: IObject,
 ) {
 	if (isCollectionOrOrderedCollection(activity)) {
-		const resolver = new Resolver();
+		apLogger.debug('Refusing to ingest collection as activity');
-		for (const item of toArray(
+		return;
-			isCollection(activity) ? activity.items : activity.orderedItems,
-		)) {
-			const act = await resolver.resolve(item);
-			try {
-				await performOneActivity(actor, act);
-			} catch (err) {
-				if (err instanceof Error || typeof err === "string") {
-					apLogger.error(err);
-				}
-			}
-		}
 	} else {
 		await performOneActivity(actor, activity);
 	}

packages/backend/src/remote/activitypub/kernel/update/index.ts

@@ -1,5 +1,5 @@
 import type { CacheableRemoteUser } from "@/models/entities/user.js";
-import type { IUpdate } from "../../type.js";
+import { getApId, IUpdate } from "../../type.js";
 import { getApType, isActor } from "../../type.js";
 import { apLogger } from "../../logger.js";
 import { updateNote } from "../../models/note.js";
@@ -13,7 +13,7 @@ export default async (
 	actor: CacheableRemoteUser,
 	activity: IUpdate,
 ): Promise<string> => {
-	if ("actor" in activity && actor.uri !== activity.actor) {
+	if (actor.uri == null || actor.uri !== getApId(activity.actor)) {
 		return "skip: invalid actor";
 	}
 
@@ -27,6 +27,10 @@ export default async (
 	});
 
 	if (isActor(object)) {
+		if (actor.uri !== object.id) {
+			return "skip: actor id mismatch";
+		}
+
 		await updatePerson(actor.uri!, resolver, object);
 		return "ok: Person updated";
 	}
@@ -39,7 +43,7 @@ export default async (
 		case "Document":
 		case "Page":
 			let failed = false;
-			await updateNote(object, resolver).catch((e: Error) => {
+			await updateNote(object, actor, resolver).catch((e: Error) => {
 				failed = true;
 			});
 			return failed ? "skip: Note update failed" : "ok: Note updated";

packages/backend/src/remote/activitypub/models/note.ts

@@ -131,13 +131,20 @@ export async function createNote(
 
 	const note: IPost = object;
 
-	if (note.id && !note.id.startsWith("https://")) {
+	if (note.id == null) {
+		throw new Error('Note must have an id');
+	}
+
+	const idUrl = new URL(note.id);
+
+	if (idUrl.protocol != 'https:') {
 		throw new Error(`unexpected schema of note.id: ${note.id}`);
 	}
 
-	const url = getOneApHrefNullable(note.url);
+	let url = getOneApHrefNullable(note.url);
+	const urlUrl = url != null ? new URL(url) : null;
 
-	if (url && !url.startsWith("https://")) {
+	if (urlUrl != null && urlUrl.protocol != 'https:') {
 		throw new Error(`unexpected schema of note url: ${url}`);
 	}
 
@@ -169,6 +176,22 @@ export async function createNote(
 		limiter
 	)) as CacheableRemoteUser;
 
+	if (actor.uri == null) {
+		logger.warn('Note actor uri is null, discarding');
+		return null;
+	}
+
+	const actorUri = new URL(actor.uri);
+	if (idUrl.host != actorUri.host) {
+		logger.warn("Note id host doesn't match actor host, discarding");
+		return null;
+	}
+
+	if (urlUrl != null && urlUrl.host != actorUri.host) {
+		logger.debug("Note url host doesn't match actor host, clearing variable");
+		url = undefined;
+	}
+
 	// Skip if author is suspended.
 	if (actor.isSuspended) {
 		logger.debug(
@@ -544,7 +567,7 @@ function notEmpty(partial: Partial<any>) {
 	return Object.keys(partial).length > 0;
 }
 
-export async function updateNote(value: string | IObject, resolver?: Resolver) {
+export async function updateNote(value: string | IObject, actor: CacheableRemoteUser, resolver?: Resolver) {
 	const uri = typeof value === "string" ? value : value.id;
 	if (!uri) throw new Error("Missing note uri");
 
@@ -557,16 +580,18 @@ export async function updateNote(value: string | IObject, resolver?: Resolver) {
 	// Resolve the updated Note object
 	const post = (await resolver.resolve(value)) as IPost;
 
-	const actor = (await resolvePerson(
+	if (getOneApId(post.attributedTo) !== actor.uri || actor.uri == null) {
-		getOneApId(post.attributedTo),
+		throw new Error('Refusing to ingest update for note with mismatching actor');
-		resolver,
+	}
-	)) as CacheableRemoteUser;
 
 	// Already registered with this server?
 	const note = await Notes.findOneBy({ uri });
 	if (note == null) {
 		return await createNote(post, resolver);
 	}
+	if (note.userId !== actor.id) {
+		throw new Error('Refusing to ingest update for note of different user');
+	}
 
 	// Whether to tell clients the note has been updated and requires refresh.
 	let updating = false;
@@ -699,6 +724,10 @@ export async function updateNote(value: string | IObject, resolver?: Resolver) {
 
 	if (poll) {
 		const dbPoll = await Polls.findOneBy({ noteId: note.id });
+		if (poll?.votes != null && poll.votes.find(p => !Number.isInteger(p) || p < 0) !== undefined) {
+			throw new Error('Refusing to ingest poll with non-integer or negative vote count');
+		}
+
 		if (dbPoll == null) {
 			await Polls.insert({
 				noteId: note.id,

packages/backend/src/remote/activitypub/models/person.ts

@@ -21,7 +21,7 @@ import { genId } from "@/misc/gen-id.js";
 import { instanceChart, usersChart } from "@/services/chart/index.js";
 import { UserPublickey } from "@/models/entities/user-publickey.js";
 import { isDuplicateKeyValueError } from "@/misc/is-duplicate-key-value-error.js";
-import { toPuny } from "@/misc/convert-host.js";
+import { extractDbHost, toPuny } from "@/misc/convert-host.js";
 import { UserProfile } from "@/models/entities/user-profile.js";
 import { toArray } from "@/prelude/array.js";
 import { fetchInstanceMetadata } from "@/services/fetch-instance-metadata.js";
@@ -69,7 +69,7 @@ const summaryLength = 2048;
  * @param uri Fetch target URI
  */
 function validateActor(x: IObject, uri: string): IActor {
-	const expectHost = toPuny(new URL(uri).hostname);
+	const expectHost = extractDbHost(uri);
 
 	if (x == null) {
 		throw new Error("invalid Actor: object is null");
@@ -83,10 +83,36 @@ function validateActor(x: IObject, uri: string): IActor {
 		throw new Error("invalid Actor: wrong id");
 	}
 
-	if (!(typeof x.inbox === "string" && x.inbox.length > 0)) {
+	if (!(typeof x.inbox === "string" && x.inbox.length > 0 && extractDbHost(x.inbox) === expectHost)) {
 		throw new Error("invalid Actor: wrong inbox");
 	}
 
+	if (!(typeof x.outbox === "string" && x.outbox.length > 0 && extractDbHost(getApId(x.outbox)) === expectHost)) {
+		throw new Error("invalid Actor: wrong outbox");
+	}
+
+	const sharedInboxObject = x.sharedInbox ?? (x.endpoints ? x.endpoints.sharedInbox : undefined);
+	if (sharedInboxObject != null) {
+		const sharedInbox = getApId(sharedInboxObject);
+		if (!(typeof sharedInbox === "string" && sharedInbox.length > 0 && extractDbHost(sharedInbox) === expectHost)) {
+			throw new Error("invalid Actor: wrong shared inbox");
+		}
+	}
+
+	if (x.followers != null) {
+		x.followers = getApId(x.followers);
+		if (!(typeof x.followers === "string" && x.followers.length > 0 && extractDbHost(x.followers) === expectHost)) {
+			throw new Error("invalid Actor: wrong followers");
+		}
+	}
+
+	if (x.following != null) {
+		x.following = getApId(x.following);
+		if (!(typeof x.following === "string" && x.following.length > 0 && extractDbHost(x.following) === expectHost)) {
+			throw new Error("invalid Actor: wrong following");
+		}
+	}
+
 	if (
 		!(
 			typeof x.preferredUsername === "string" &&
@@ -114,7 +140,7 @@ function validateActor(x: IObject, uri: string): IActor {
 		x.summary = truncate(x.summary, summaryLength);
 	}
 
-	const idHost = toPuny(new URL(x.id!).hostname);
+	const idHost = toPuny(new URL(x.id!).host);
 	if (idHost !== expectHost) {
 		throw new Error("invalid Actor: id has different host");
 	}
@@ -124,7 +150,7 @@ function validateActor(x: IObject, uri: string): IActor {
 			throw new Error("invalid Actor: publicKey.id is not a string");
 		}
 
-		const publicKeyIdHost = toPuny(new URL(x.publicKey.id).hostname);
+		const publicKeyIdHost = toPuny(new URL(x.publicKey.id).host);
 		if (publicKeyIdHost !== expectHost) {
 			throw new Error("invalid Actor: publicKey.id has different host");
 		}
@@ -195,10 +221,10 @@ export async function createPerson(
 		person = validateActor(object, uri);
 	}
 	catch (e: any) {
-		if (typeof object.publicKey?.owner !== 'string')
+		// Work around GoToSocial issue #1186 (ref: https://github.com/superseriousbusiness/gotosocial/issues/1186)
+		if (typeof object.publicKey?.owner !== 'string' || object.inbox != null)
 			throw e;
 
-		// Work around GoToSocial issue #1186 (ref: https://github.com/superseriousbusiness/gotosocial/issues/1186)
 		logger.info(`Received stub actor, re-resolving with key owner uri: ${object.publicKey.owner}`);
 		object = (await resolver.resolve(object.publicKey.owner)) as any;
 		person = validateActor(object, uri);
@@ -261,12 +287,19 @@ export async function createPerson(
 
 	const bday = person["vcard:bday"]?.match(/^\d{4}-\d{2}-\d{2}/);
 
-	const url = getOneApHrefNullable(person.url);
+	let url = getOneApHrefNullable(person.url);
+	const urlUrl = url != null ? new URL(url) : null;
+	const uriUrl = new URL(uri);
 
-	if (url && !url.startsWith("https://")) {
+	if (urlUrl != null && urlUrl.protocol != 'https:') {
 		throw new Error(`unexpected schema of person url: ${url}`);
 	}
 
+	if (urlUrl != null && urlUrl.host != uriUrl.host) {
+		logger.debug("Person url host doesn't match person uri host, clearing variable");
+		url = undefined;
+	}
+
 	let followersCount: number | undefined;
 
 	if (typeof person.followers === "string") {

packages/backend/src/server/activitypub.ts

@@ -51,7 +51,7 @@ function inbox(ctx: Router.RouterContext) {
 	let signature;
 
 	try {
-		signature = httpSignature.parseRequest(ctx.req, { headers: ['(request-target)', 'digest', 'host', 'date'] });
+		signature = httpSignature.parseRequest(ctx.req, { headers: ['(request-target)', 'digest', 'host', 'date'], authorizationHeaderName: 'signature' });
 	} catch (e) {
 		ctx.status = 401;
 		return;

packages/client/src/pages/user/home.vue

@@ -15,7 +15,7 @@
 					/>
 					<MkRemoteCaution
 						v-if="user.host != null"
-						:href="user.url"
+						:href="user.url ?? user.uri"
 						class="warn"
 					/>
 

packages/client/src/scripts/get-user-menu.ts

@@ -293,7 +293,7 @@ export function getUserMenu(user, router: Router = mainRouter) {
 					type: "a",
 					icon: "ph-arrow-square-out ph-bold ph-lg",
 					text: i18n.ts.showOnRemote,
-					href: user.url,
+					href: user.url ?? user.uri,
 					target: "_blank",
 			  }
 			: undefined,