From 66513b9893feedd7bc35c049e4e74f78cfde089e Mon Sep 17 00:00:00 2001 From: Derek Date: Fri, 2 Dec 2022 16:13:36 -0500 Subject: [PATCH] fix(server): "forkbomb" DOS mitigation (#9247) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add recursion limit to resolver * Use shared resolver in featured and question * Changelog * Changelog fix * Update CHANGELOG.md Co-authored-by: Acid Chicken (硫酸鶏) * Add host to recursion limit error message Co-authored-by: Acid Chicken (硫酸鶏) Co-authored-by: syuilo Co-authored-by: Acid Chicken (硫酸鶏) --- CHANGELOG.md | 3 ++- .../backend/src/core/remote/activitypub/ApInboxService.ts | 2 +- .../src/core/remote/activitypub/ApResolverService.ts | 5 +++++ .../src/core/remote/activitypub/models/ApPersonService.ts | 8 ++++---- .../core/remote/activitypub/models/ApQuestionService.ts | 4 ++-- 5 files changed, 14 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ab5c3ed4d5..38b1546802 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ ### Improvements ### Bugfixes -- +- You should also include the user name that made the change. --> @@ -25,6 +25,7 @@ You should also include the user name that made the change. - Server: 引用内の文章がnyaizeされてしまう問題を修正 @kabo2468 - Server: Bug fix for Pinned Users lookup on instance @squidicuzz - Client: インスタンスティッカーのfaviconを読み込む際に偽サイト警告が出ることがあるのを修正 @syuilo +- Server: Mitigate AP reference chain DoS vector @skehmatics ## 12.119.0 (2022/09/10) diff --git a/packages/backend/src/core/remote/activitypub/ApInboxService.ts b/packages/backend/src/core/remote/activitypub/ApInboxService.ts index baeeb1ad3e..3da384ec2d 100644 --- a/packages/backend/src/core/remote/activitypub/ApInboxService.ts +++ b/packages/backend/src/core/remote/activitypub/ApInboxService.ts @@ -731,7 +731,7 @@ export class ApInboxService { await this.apPersonService.updatePerson(actor.uri!, resolver, object); return 'ok: Person updated'; } else if (getApType(object) === 'Question') { - await this.apQuestionService.updateQuestion(object).catch(err => console.error(err)); + await this.apQuestionService.updateQuestion(object, resolver).catch(err => console.error(err)); return 'ok: Question updated'; } else { return `skip: Unknown type: ${getApType(object)}`; diff --git a/packages/backend/src/core/remote/activitypub/ApResolverService.ts b/packages/backend/src/core/remote/activitypub/ApResolverService.ts index fe57c82ccc..bcdb9383d1 100644 --- a/packages/backend/src/core/remote/activitypub/ApResolverService.ts +++ b/packages/backend/src/core/remote/activitypub/ApResolverService.ts @@ -76,6 +76,7 @@ export class Resolver { private httpRequestService: HttpRequestService, private apRendererService: ApRendererService, private apDbResolverService: ApDbResolverService, + private recursionLimit = 100 ) { this.history = new Set(); } @@ -116,6 +117,10 @@ export class Resolver { throw new Error('cannot resolve already resolved one'); } + if (this.history.size > this.recursionLimit) { + throw new Error(`hit recursion limit: ${this.utilityService.extractDbHost(value)}`); + } + this.history.add(value); const host = this.utilityService.extractDbHost(value); diff --git a/packages/backend/src/core/remote/activitypub/models/ApPersonService.ts b/packages/backend/src/core/remote/activitypub/models/ApPersonService.ts index 5135473862..f9d6f42ef6 100644 --- a/packages/backend/src/core/remote/activitypub/models/ApPersonService.ts +++ b/packages/backend/src/core/remote/activitypub/models/ApPersonService.ts @@ -390,7 +390,7 @@ export class ApPersonService implements OnModuleInit { }); //#endregion - await this.updateFeatured(user!.id).catch(err => this.logger.error(err)); + await this.updateFeatured(user!.id, resolver).catch(err => this.logger.error(err)); return user!; } @@ -503,7 +503,7 @@ export class ApPersonService implements OnModuleInit { followerSharedInbox: person.sharedInbox ?? (person.endpoints ? person.endpoints.sharedInbox : undefined), }); - await this.updateFeatured(exist.id).catch(err => this.logger.error(err)); + await this.updateFeatured(exist.id, resolver).catch(err => this.logger.error(err)); } /** @@ -551,14 +551,14 @@ export class ApPersonService implements OnModuleInit { return { fields, services }; } - public async updateFeatured(userId: User['id']) { + public async updateFeatured(userId: User['id'], resolver?: Resolver) { const user = await this.usersRepository.findOneByOrFail({ id: userId }); if (!this.userEntityService.isRemoteUser(user)) return; if (!user.featured) return; this.logger.info(`Updating the featured: ${user.uri}`); - const resolver = this.apResolverService.createResolver(); + if (resolver == null) resolver = this.apResolverService.createResolver(); // Resolve to (Ordered)Collection Object const collection = await resolver.resolveCollection(user.featured); diff --git a/packages/backend/src/core/remote/activitypub/models/ApQuestionService.ts b/packages/backend/src/core/remote/activitypub/models/ApQuestionService.ts index acd5cdae83..5793b98353 100644 --- a/packages/backend/src/core/remote/activitypub/models/ApQuestionService.ts +++ b/packages/backend/src/core/remote/activitypub/models/ApQuestionService.ts @@ -65,7 +65,7 @@ export class ApQuestionService { * @param uri URI of AP Question object * @returns true if updated */ - public async updateQuestion(value: any) { + public async updateQuestion(value: any, resolver?: Resolver) { const uri = typeof value === 'string' ? value : value.id; // URIがこのサーバーを指しているならスキップ @@ -80,7 +80,7 @@ export class ApQuestionService { //#endregion // resolve new Question object - const resolver = this.apResolverService.createResolver(); + if (resolver == null) resolver = this.apResolverService.createResolver(); const question = await resolver.resolve(value) as IQuestion; this.logger.debug(`fetched question: ${JSON.stringify(question, null, 2)}`);