From 2c69cb4a926d20f2e26e14f0601ce3bdf30ddc90 Mon Sep 17 00:00:00 2001 From: Johann150 Date: Wed, 16 Aug 2023 19:21:37 +0200 Subject: [PATCH] server: check inbox URLs This adds a check for inbox and sharedInbox URLs to be both valid and also absolute URLs. If the normal inbox URL is invalid, the actor will be rejected. If the sharedInbox URL is invalid, it will be ignored. Changelog: Fixed --- .../src/remote/activitypub/models/person.ts | 26 +++++++++++++++---- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/packages/backend/src/remote/activitypub/models/person.ts b/packages/backend/src/remote/activitypub/models/person.ts index 3fe042a82..e354f8811 100644 --- a/packages/backend/src/remote/activitypub/models/person.ts +++ b/packages/backend/src/remote/activitypub/models/person.ts @@ -1,3 +1,4 @@ +import { URL } from 'node:url'; import promiseLimit from 'promise-limit'; import { Not, IsNull } from 'typeorm'; @@ -77,8 +78,23 @@ async function validateActor(x: IObject, resolver: Resolver): Promise { }); } - if (!(typeof x.inbox === 'string' && x.inbox.length > 0)) { - throw new Error('invalid Actor: wrong inbox'); + // check that inbox is a valid and absolute URL + // in NodeJS, the first parameter must be an absolute URL or the base URL is required + try { + new URL(x.inbox) + } catch (err) { + throw new Error('invalid Actor: wrong inbox', { cause: err }); + } + + // unify different sharedInbox places + x.sharedInbox = x.sharedInbox ?? x.endpoints?.sharedInbox; + if (x.sharedInbox != null) { + // check that sharedInbox is a valid and absolute URL + try { + new URL(x.sharedInbox); + } catch (err) { + delete x.sharedInbox; + } } if (!(typeof x.preferredUsername === 'string' && x.preferredUsername.length > 0 && x.preferredUsername.length <= 128 && /^\w([\w-.]*\w)?$/.test(x.preferredUsername))) { @@ -185,7 +201,7 @@ export async function createPerson(value: string | IObject, resolver: Resolver): usernameLower: person.preferredUsername!.toLowerCase(), host, inbox: person.inbox, - sharedInbox: person.sharedInbox || (person.endpoints ? person.endpoints.sharedInbox : undefined), + sharedInbox: person.sharedInbox, followersUri: person.followers ? getApId(person.followers) : undefined, featured: person.featured ? getApId(person.featured) : undefined, uri: person.id, @@ -335,7 +351,7 @@ export async function updatePerson(value: IObject | string, resolver: Resolver): const updates = { lastFetchedAt: new Date(), inbox: person.inbox, - sharedInbox: person.sharedInbox ?? (person.endpoints ? person.endpoints.sharedInbox : undefined), + sharedInbox: person.sharedInbox, followersUri: person.followers ? getApId(person.followers) : undefined, featured: person.featured, emojis: emojiNames, @@ -382,7 +398,7 @@ export async function updatePerson(value: IObject | string, resolver: Resolver): await Followings.update({ followerId: exist.id, }, { - followerSharedInbox: person.sharedInbox ?? (person.endpoints ? person.endpoints.sharedInbox : undefined), + followerSharedInbox: person.sharedInbox, }); await updateFeatured(exist.id, resolver).catch(err => apLogger.error(err));