From 5f675201f261d5db6a58d3099a190372bb2f09f0 Mon Sep 17 00:00:00 2001 From: Julia Date: Wed, 20 Nov 2024 18:20:09 -0500 Subject: Merge commit from fork * enhance: Add a few validation fixes from Sharkey See the original MR on the GitLab instance: https://activitypub.software/TransFem-org/Sharkey/-/merge_requests/484 Co-Authored-By: Dakkar * fix: primitive 2: acceptance of cross-origin alternate Co-Authored-By: Laura Hausmann * fix: primitive 3: validation of non-final url * fix: primitive 4: missing same-origin identifier validation of collection-wrapped activities * fix: primitives 5 & 8: reject activities with non string identifiers Co-Authored-By: Laura Hausmann * fix: primitive 6: reject anonymous objects that were fetched by their id * fix: primitives 9, 10 & 11: http signature validation doesn't enforce required headers or specify auth header name Co-Authored-By: Laura Hausmann * fix: primitive 14: improper validation of outbox, followers, following & shared inbox collections * fix: code style for primitive 14 * fix: primitive 15: improper same-origin validation for note uri and url Co-Authored-By: Laura Hausmann * fix: primitive 16: improper same-origin validation for user uri and url * fix: primitive 17: note same-origin identifier validation can be bypassed by wrapping the id in an array * fix: code style for primitive 17 * fix: check attribution against actor in notes While this isn't strictly required to fix the exploits at hand, this mirrors the fix in `ApQuestionService` for GHSA-5h8r-gq97-xv69, as a preemptive countermeasure. * fix: primitive 18: `ap/get` bypasses access checks One might argue that we could make this one actually preform access checks against the returned activity object, but I feel like that's a lot more work than just restricting it to administrators, since, to me at least, it seems more like a debugging tool than anything else. * fix: primitive 19 & 20: respect blocks and hide more Ideally, the user property should also be hidden (as leaving it in leaks information slightly), but given the schema of the note endpoint, I don't think that would be possible without introducing some kind of "ghost" user, who is attributed for posts by users who have you blocked. * fix: primitives 21, 22, and 23: reuse resolver This also increases the default `recursionLimit` for `Resolver`, as it theoretically will go higher that it previously would and could possibly fail on non-malicious collection activities. * fix: primitives 25-33: proper local instance checks * revert: fix: primitive 19 & 20 This reverts commit 465a9fe6591de90f78bd3d084e3c01e65dc3cf3c. --------- Co-authored-by: Dakkar Co-authored-by: Laura Hausmann Co-authored-by: syuilo <4439005+syuilo@users.noreply.github.com> --- packages/backend/src/server/api/endpoints/ap/get.ts | 1 + packages/backend/src/server/api/endpoints/ap/show.ts | 5 +++++ 2 files changed, 6 insertions(+) (limited to 'packages/backend/src/server/api/endpoints/ap') diff --git a/packages/backend/src/server/api/endpoints/ap/get.ts b/packages/backend/src/server/api/endpoints/ap/get.ts index d8c55de7ec..14286bc23e 100644 --- a/packages/backend/src/server/api/endpoints/ap/get.ts +++ b/packages/backend/src/server/api/endpoints/ap/get.ts @@ -11,6 +11,7 @@ import { ApResolverService } from '@/core/activitypub/ApResolverService.js'; export const meta = { tags: ['federation'], + requireAdmin: true, requireCredential: true, kind: 'read:federation', diff --git a/packages/backend/src/server/api/endpoints/ap/show.ts b/packages/backend/src/server/api/endpoints/ap/show.ts index c52608cefb..aca8258c08 100644 --- a/packages/backend/src/server/api/endpoints/ap/show.ts +++ b/packages/backend/src/server/api/endpoints/ap/show.ts @@ -118,6 +118,11 @@ export default class extends Endpoint { // eslint- ])); if (local != null) return local; + const host = this.utilityService.extractDbHost(uri); + + // local object, not found in db? fail + if (this.utilityService.isSelfHost(host)) return null; + // リモートから一旦オブジェクトフェッチ const resolver = this.apResolverService.createResolver(); const object = await resolver.resolve(uri) as any; -- cgit v1.2.3-freya From 0f59adc436f80c495b4404807b0bd645da2b1db8 Mon Sep 17 00:00:00 2001 From: syuilo <4439005+syuilo@users.noreply.github.com> Date: Thu, 21 Nov 2024 09:25:18 +0900 Subject: fix ap/show --- packages/backend/src/server/api/endpoints/ap/show.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'packages/backend/src/server/api/endpoints/ap') diff --git a/packages/backend/src/server/api/endpoints/ap/show.ts b/packages/backend/src/server/api/endpoints/ap/show.ts index aca8258c08..5c1d8dce9f 100644 --- a/packages/backend/src/server/api/endpoints/ap/show.ts +++ b/packages/backend/src/server/api/endpoints/ap/show.ts @@ -140,7 +140,7 @@ export default class extends Endpoint { // eslint- return await this.mergePack( me, isActor(object) ? await this.apPersonService.createPerson(getApId(object)) : null, - isPost(object) ? await this.apNoteService.createNote(getApId(object), undefined, true) : null, + isPost(object) ? await this.apNoteService.createNote(getApId(object), undefined, resolver) : null, ); } -- cgit v1.2.3-freya From 53e827b18c46f786268278645206404ff2d95f72 Mon Sep 17 00:00:00 2001 From: かっこかり <67428053+kakkokari-gtyih@users.noreply.github.com> Date: Thu, 21 Nov 2024 10:30:30 +0900 Subject: fix(backend): fix security patches (#15008) --- packages/backend/src/core/activitypub/ApInboxService.ts | 2 +- packages/backend/src/server/api/endpoints/ap/show.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'packages/backend/src/server/api/endpoints/ap') diff --git a/packages/backend/src/core/activitypub/ApInboxService.ts b/packages/backend/src/core/activitypub/ApInboxService.ts index 72f5eeda14..9838e3bd30 100644 --- a/packages/backend/src/core/activitypub/ApInboxService.ts +++ b/packages/backend/src/core/activitypub/ApInboxService.ts @@ -130,7 +130,7 @@ export class ApInboxService { if (actor.uri) { if (actor.lastFetchedAt == null || Date.now() - actor.lastFetchedAt.getTime() > 1000 * 60 * 60 * 24) { setImmediate(() => { - this.apPersonService.updatePerson(actor.uri); + this.apPersonService.updatePerson(actor.uri, resolver); }); } } diff --git a/packages/backend/src/server/api/endpoints/ap/show.ts b/packages/backend/src/server/api/endpoints/ap/show.ts index 5c1d8dce9f..bf99834c17 100644 --- a/packages/backend/src/server/api/endpoints/ap/show.ts +++ b/packages/backend/src/server/api/endpoints/ap/show.ts @@ -139,8 +139,8 @@ export default class extends Endpoint { // eslint- return await this.mergePack( me, - isActor(object) ? await this.apPersonService.createPerson(getApId(object)) : null, - isPost(object) ? await this.apNoteService.createNote(getApId(object), undefined, resolver) : null, + isActor(object) ? await this.apPersonService.createPerson(getApId(object), resolver) : null, + isPost(object) ? await this.apNoteService.createNote(getApId(object), undefined, resolver, true) : null, ); } -- cgit v1.2.3-freya From c1f19fad1e7e1717898b37bbb4e863e0f26b306b Mon Sep 17 00:00:00 2001 From: かっこかり <67428053+kakkokari-gtyih@users.noreply.github.com> Date: Thu, 21 Nov 2024 14:36:24 +0900 Subject: fix(backend): fix apResolver (#15010) * fix(backend): fix apResolver * fix * add comments * tweak comment --- packages/backend/src/core/activitypub/ApInboxService.ts | 3 ++- packages/backend/src/server/api/endpoints/ap/show.ts | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) (limited to 'packages/backend/src/server/api/endpoints/ap') diff --git a/packages/backend/src/core/activitypub/ApInboxService.ts b/packages/backend/src/core/activitypub/ApInboxService.ts index 9838e3bd30..ee3f691c51 100644 --- a/packages/backend/src/core/activitypub/ApInboxService.ts +++ b/packages/backend/src/core/activitypub/ApInboxService.ts @@ -130,7 +130,8 @@ export class ApInboxService { if (actor.uri) { if (actor.lastFetchedAt == null || Date.now() - actor.lastFetchedAt.getTime() > 1000 * 60 * 60 * 24) { setImmediate(() => { - this.apPersonService.updatePerson(actor.uri, resolver); + // 同一ユーザーの情報を再度処理するので、使用済みのresolverを再利用してはいけない + this.apPersonService.updatePerson(actor.uri); }); } } diff --git a/packages/backend/src/server/api/endpoints/ap/show.ts b/packages/backend/src/server/api/endpoints/ap/show.ts index bf99834c17..24d5a7b0f1 100644 --- a/packages/backend/src/server/api/endpoints/ap/show.ts +++ b/packages/backend/src/server/api/endpoints/ap/show.ts @@ -137,10 +137,11 @@ export default class extends Endpoint { // eslint- if (local != null) return local; } + // 同一ユーザーの情報を再度処理するので、使用済みのresolverを再利用してはいけない return await this.mergePack( me, - isActor(object) ? await this.apPersonService.createPerson(getApId(object), resolver) : null, - isPost(object) ? await this.apNoteService.createNote(getApId(object), undefined, resolver, true) : null, + isActor(object) ? await this.apPersonService.createPerson(getApId(object)) : null, + isPost(object) ? await this.apNoteService.createNote(getApId(object), undefined, undefined, true) : null, ); } -- cgit v1.2.3-freya