From 4f98fa84612b4bd6d1c341276a0018dc36422301 Mon Sep 17 00:00:00 2001 From: Laura Hausmann Date: Wed, 14 Aug 2024 03:44:14 +0200 Subject: [PATCH] [backend/federation] Fix possibly unbounded UserResolver recursion --- .../Controllers/Mastodon/AccountController.cs | 2 +- .../Controllers/Mastodon/SearchController.cs | 4 +- .../Controllers/Web/SearchController.cs | 4 +- .../Core/Extensions/MvcBuilderExtensions.cs | 3 +- .../ActivityPub/ActivityHandlerService.cs | 12 ++--- .../Federation/ActivityPub/UserResolver.cs | 46 ++++++++++--------- .../Middleware/AuthorizedFetchMiddleware.cs | 2 +- .../Middleware/InboxValidationMiddleware.cs | 4 +- .../Core/Services/NoteService.cs | 10 ++-- .../Services/UserProfileMentionsResolver.cs | 8 ++-- 10 files changed, 48 insertions(+), 47 deletions(-) diff --git a/Iceshrimp.Backend/Controllers/Mastodon/AccountController.cs b/Iceshrimp.Backend/Controllers/Mastodon/AccountController.cs index aecc1ea5..1a3ab7c0 100644 --- a/Iceshrimp.Backend/Controllers/Mastodon/AccountController.cs +++ b/Iceshrimp.Backend/Controllers/Mastodon/AccountController.cs @@ -604,7 +604,7 @@ public class AccountController( [ProducesErrors(HttpStatusCode.NotFound)] public async Task LookupUser([FromQuery] string acct) { - var user = await userResolver.LookupAsync(acct) ?? throw GracefulException.RecordNotFound(); + var user = await userResolver.LookupAsync(acct, false) ?? throw GracefulException.RecordNotFound(); return await userRenderer.RenderAsync(user); } diff --git a/Iceshrimp.Backend/Controllers/Mastodon/SearchController.cs b/Iceshrimp.Backend/Controllers/Mastodon/SearchController.cs index ae4ca2ea..1f94af91 100644 --- a/Iceshrimp.Backend/Controllers/Mastodon/SearchController.cs +++ b/Iceshrimp.Backend/Controllers/Mastodon/SearchController.cs @@ -86,7 +86,7 @@ public class SearchController( if (pagination.Offset is not null and not 0) return []; try { - var result = await userResolver.ResolveAsync(search.Query); + var result = await userResolver.ResolveAsync(search.Query, false); return [await userRenderer.RenderAsync(result)]; } catch @@ -118,7 +118,7 @@ public class SearchController( try { - var result = await userResolver.ResolveAsync($"@{username}@{host}"); + var result = await userResolver.ResolveAsync($"@{username}@{host}", false); return [await userRenderer.RenderAsync(result)]; } catch diff --git a/Iceshrimp.Backend/Controllers/Web/SearchController.cs b/Iceshrimp.Backend/Controllers/Web/SearchController.cs index 4167a798..5277e74b 100644 --- a/Iceshrimp.Backend/Controllers/Web/SearchController.cs +++ b/Iceshrimp.Backend/Controllers/Web/SearchController.cs @@ -87,7 +87,7 @@ public class SearchController( if (target.StartsWith('@') || target.StartsWith(userPrefixAlt)) { - var hit = await userResolver.ResolveAsyncOrNull(target); + var hit = await userResolver.ResolveAsyncOrNull(target, false); if (hit != null) return new RedirectResponse { TargetUrl = $"/users/{hit.Id}" }; throw GracefulException.NotFound("No result found"); } @@ -125,7 +125,7 @@ public class SearchController( noteHit = await noteSvc.ResolveNoteAsync(target); if (noteHit != null) return new RedirectResponse { TargetUrl = $"/notes/{noteHit.Id}" }; - userHit = await userResolver.ResolveAsyncOrNull(target); + userHit = await userResolver.ResolveAsyncOrNull(target, false); if (userHit != null) return new RedirectResponse { TargetUrl = $"/users/{userHit.Id}" }; throw GracefulException.NotFound("No result found"); diff --git a/Iceshrimp.Backend/Core/Extensions/MvcBuilderExtensions.cs b/Iceshrimp.Backend/Core/Extensions/MvcBuilderExtensions.cs index e802167f..1ab78521 100644 --- a/Iceshrimp.Backend/Core/Extensions/MvcBuilderExtensions.cs +++ b/Iceshrimp.Backend/Core/Extensions/MvcBuilderExtensions.cs @@ -112,9 +112,8 @@ public class AcceptHeaderOutputFormatterSelector( ) : OutputFormatterSelector { private readonly DefaultOutputFormatterSelector _fallbackSelector = new(options, loggerFactory); - private readonly List _formatters = [..options.Value.OutputFormatters]; - public override IOutputFormatter? SelectFormatter( + public override IOutputFormatter SelectFormatter( OutputFormatterCanWriteContext context, IList formatters, MediaTypeCollection mediaTypes ) { diff --git a/Iceshrimp.Backend/Core/Federation/ActivityPub/ActivityHandlerService.cs b/Iceshrimp.Backend/Core/Federation/ActivityPub/ActivityHandlerService.cs index fc06a46f..a7550ec9 100644 --- a/Iceshrimp.Backend/Core/Federation/ActivityPub/ActivityHandlerService.cs +++ b/Iceshrimp.Backend/Core/Federation/ActivityPub/ActivityHandlerService.cs @@ -39,7 +39,7 @@ public class ActivityHandlerService( if (activity.Object == null && activity is not ASBite) throw GracefulException.UnprocessableEntity("Activity object is null"); - var resolvedActor = await userResolver.ResolveAsync(activity.Actor.Id); + var resolvedActor = await userResolver.ResolveAsync(activity.Actor.Id, true); if (authenticatedUserId == null) throw GracefulException.UnprocessableEntity("Refusing to process activity without authenticatedUserId"); @@ -157,7 +157,7 @@ public class ActivityHandlerService( if (activity.Object is not ASActor obj) throw GracefulException.UnprocessableEntity("Follow activity object is invalid"); - var followee = await userResolver.ResolveAsync(obj.Id); + var followee = await userResolver.ResolveAsync(obj.Id, true); if (followee.IsRemoteUser) throw GracefulException.UnprocessableEntity("Cannot process follow for remote followee"); @@ -223,7 +223,7 @@ public class ActivityHandlerService( if (follow is not { Actor: not null }) throw GracefulException.UnprocessableEntity("Refusing to reject object with invalid follow object"); - var resolvedFollower = await userResolver.ResolveAsync(follow.Actor.Id); + var resolvedFollower = await userResolver.ResolveAsync(follow.Actor.Id, true); if (resolvedFollower is not { IsLocalUser: true }) throw GracefulException.UnprocessableEntity("Refusing to reject remote follow"); if (resolvedActor.Uri == null) @@ -355,7 +355,7 @@ public class ActivityHandlerService( Uri = activity.Id, User = resolvedActor, UserHost = resolvedActor.Host, - TargetUser = await userResolver.ResolveAsync(targetActor.Id) + TargetUser = await userResolver.ResolveAsync(targetActor.Id, true) }, ASNote targetNote => new Bite { @@ -385,7 +385,7 @@ public class ActivityHandlerService( Uri = activity.Id, User = resolvedActor, UserHost = resolvedActor.Host, - TargetUser = await userResolver.ResolveAsync(activity.To.Id) + TargetUser = await userResolver.ResolveAsync(activity.To.Id, true) }, _ => throw GracefulException.UnprocessableEntity($"Invalid bite target {target.Id} with type {target.Type}") @@ -479,7 +479,7 @@ public class ActivityHandlerService( private async Task UnfollowAsync(ASActor followeeActor, User follower) { //TODO: send reject? or do we not want to copy that part of the old ap core - var followee = await userResolver.ResolveAsync(followeeActor.Id); + var followee = await userResolver.ResolveAsync(followeeActor.Id, true); await db.FollowRequests.Where(p => p.Follower == follower && p.Followee == followee).ExecuteDeleteAsync(); diff --git a/Iceshrimp.Backend/Core/Federation/ActivityPub/UserResolver.cs b/Iceshrimp.Backend/Core/Federation/ActivityPub/UserResolver.cs index ea17f668..3ac3d202 100644 --- a/Iceshrimp.Backend/Core/Federation/ActivityPub/UserResolver.cs +++ b/Iceshrimp.Backend/Core/Federation/ActivityPub/UserResolver.cs @@ -192,21 +192,23 @@ public class UserResolver( return query; } - public async Task ResolveAsync(string username, string? host) + public async Task ResolveAsync(string username, string? host, bool skipUpdate) { - return host != null ? await ResolveAsync($"acct:{username}@{host}") : await ResolveAsync($"acct:{username}"); + return host != null + ? await ResolveAsync($"acct:{username}@{host}", skipUpdate) + : await ResolveAsync($"acct:{username}", skipUpdate); } - public async Task LookupAsync(string query) + public async Task LookupAsync(string query, bool skipUpdate) { query = NormalizeQuery(query); var user = await userSvc.GetUserFromQueryAsync(query); if (user != null) - return await GetUpdatedUser(user); + return skipUpdate ? user : await GetUpdatedUser(user); return user; } - public async Task ResolveAsync(string query) + public async Task ResolveAsync(string query, bool skipUpdate) { query = NormalizeQuery(query); @@ -217,16 +219,16 @@ public class UserResolver( // First, let's see if we already know the user var user = await userSvc.GetUserFromQueryAsync(query); if (user != null) - return await GetUpdatedUser(user); + return skipUpdate ? user : await GetUpdatedUser(user); // We don't, so we need to run WebFinger var (acct, uri) = await WebFingerAsync(query); // Check the database again with the new data - if (uri != query) user = await userSvc.GetUserFromQueryAsync(uri); - if (user == null && acct != query) await userSvc.GetUserFromQueryAsync(acct); + if (uri != query) user = await userSvc.GetUserFromQueryAsync(uri); + if (user == null && acct != query) user = await userSvc.GetUserFromQueryAsync(acct); if (user != null) - return await GetUpdatedUser(user); + return skipUpdate ? user : await GetUpdatedUser(user); using (await KeyedLocker.LockAsync(uri)) { @@ -235,7 +237,7 @@ public class UserResolver( } } - public async Task ResolveAsync(string query, bool onlyExisting) + public async Task ResolveAsync(string query, bool onlyExisting, bool skipUpdate) { query = NormalizeQuery(query); @@ -246,7 +248,7 @@ public class UserResolver( // First, let's see if we already know the user var user = await userSvc.GetUserFromQueryAsync(query); if (user != null) - return await GetUpdatedUser(user); + return skipUpdate ? user : await GetUpdatedUser(user); if (onlyExisting) return null; @@ -255,10 +257,10 @@ public class UserResolver( var (acct, uri) = await WebFingerAsync(query); // Check the database again with the new data - if (uri != query) user = await userSvc.GetUserFromQueryAsync(uri); - if (user == null && acct != query) await userSvc.GetUserFromQueryAsync(acct); + if (uri != query) user = await userSvc.GetUserFromQueryAsync(uri); + if (user == null && acct != query) user = await userSvc.GetUserFromQueryAsync(acct); if (user != null) - return await GetUpdatedUser(user); + return skipUpdate ? user : await GetUpdatedUser(user); using (await KeyedLocker.LockAsync(uri)) { @@ -267,7 +269,7 @@ public class UserResolver( } } - public async Task ResolveAsyncOrNull(string username, string? host) + public async Task ResolveAsyncOrNull(string username, string? host, bool skipUpdate) { try { @@ -276,7 +278,7 @@ public class UserResolver( // First, let's see if we already know the user var user = await userSvc.GetUserFromQueryAsync(query); if (user != null) - return await GetUpdatedUser(user); + return skipUpdate ? user : await GetUpdatedUser(user); if (host == null) return null; @@ -284,10 +286,10 @@ public class UserResolver( var (acct, uri) = await WebFingerAsync(query); // Check the database again with the new data - if (uri != query) user = await userSvc.GetUserFromQueryAsync(uri); - if (user == null && acct != query) await userSvc.GetUserFromQueryAsync(acct); + if (uri != query) user = await userSvc.GetUserFromQueryAsync(uri); + if (user == null && acct != query) user = await userSvc.GetUserFromQueryAsync(acct); if (user != null) - return await GetUpdatedUser(user); + return skipUpdate ? user : await GetUpdatedUser(user); using (await KeyedLocker.LockAsync(uri)) { @@ -301,7 +303,7 @@ public class UserResolver( } } - public async Task ResolveAsyncOrNull(string query) + public async Task ResolveAsyncOrNull(string query, bool skipUpdate) { try { @@ -310,7 +312,7 @@ public class UserResolver( // First, let's see if we already know the user var user = await userSvc.GetUserFromQueryAsync(query); if (user != null) - return await GetUpdatedUser(user); + return skipUpdate ? user : await GetUpdatedUser(user); if (query.StartsWith($"https://{config.Value.WebDomain}/")) return null; @@ -321,7 +323,7 @@ public class UserResolver( if (resolvedUri != query) user = await userSvc.GetUserFromQueryAsync(resolvedUri); if (user == null && acct != query) await userSvc.GetUserFromQueryAsync(acct); if (user != null) - return await GetUpdatedUser(user); + return skipUpdate ? user : await GetUpdatedUser(user); using (await KeyedLocker.LockAsync(resolvedUri)) { diff --git a/Iceshrimp.Backend/Core/Middleware/AuthorizedFetchMiddleware.cs b/Iceshrimp.Backend/Core/Middleware/AuthorizedFetchMiddleware.cs index c9ceacb8..0fd4afc6 100644 --- a/Iceshrimp.Backend/Core/Middleware/AuthorizedFetchMiddleware.cs +++ b/Iceshrimp.Backend/Core/Middleware/AuthorizedFetchMiddleware.cs @@ -66,7 +66,7 @@ public class AuthorizedFetchMiddleware( { try { - var user = await userResolver.ResolveAsync(sig.KeyId).WaitAsync(ct); + var user = await userResolver.ResolveAsync(sig.KeyId, skipUpdate: true).WaitAsync(ct); key = await db.UserPublickeys.Include(p => p.User) .FirstOrDefaultAsync(p => p.User == user, ct); diff --git a/Iceshrimp.Backend/Core/Middleware/InboxValidationMiddleware.cs b/Iceshrimp.Backend/Core/Middleware/InboxValidationMiddleware.cs index a94c6f7f..7b8ef788 100644 --- a/Iceshrimp.Backend/Core/Middleware/InboxValidationMiddleware.cs +++ b/Iceshrimp.Backend/Core/Middleware/InboxValidationMiddleware.cs @@ -108,7 +108,7 @@ public class InboxValidationMiddleware( { try { - var user = await userResolver.ResolveAsync(sig.KeyId, activity is ASDelete).WaitAsync(ct); + var user = await userResolver.ResolveAsync(sig.KeyId, activity is ASDelete, true).WaitAsync(ct); if (user == null) throw AuthFetchException.NotFound("Delete activity actor is unknown"); key = await db.UserPublickeys.Include(p => p.User) .FirstOrDefaultAsync(p => p.User == user, ct); @@ -185,7 +185,7 @@ public class InboxValidationMiddleware( if (key == null) { - var user = await userResolver.ResolveAsync(activity.Actor.Id, activity is ASDelete) + var user = await userResolver.ResolveAsync(activity.Actor.Id, activity is ASDelete, true) .WaitAsync(ct); if (user == null) throw AuthFetchException.NotFound("Delete activity actor is unknown"); key = await db.UserPublickeys diff --git a/Iceshrimp.Backend/Core/Services/NoteService.cs b/Iceshrimp.Backend/Core/Services/NoteService.cs index 36f1c5f4..606d0d0f 100644 --- a/Iceshrimp.Backend/Core/Services/NoteService.cs +++ b/Iceshrimp.Backend/Core/Services/NoteService.cs @@ -177,7 +177,7 @@ public class NoteService( if (asNote != null) { visibleUserIds = (await asNote.GetRecipients(user) - .Select(userResolver.ResolveAsync) + .Select(p => userResolver.ResolveAsync(p, true)) .AwaitAllNoConcurrencyAsync()) .Select(p => p.Id) .Concat(mentionedUserIds) @@ -480,7 +480,7 @@ public class NoteService( if (asNote != null) { visibleUserIds = (await asNote.GetRecipients(note.User) - .Select(userResolver.ResolveAsync) + .Select(p => userResolver.ResolveAsync(p, true)) .AwaitAllNoConcurrencyAsync()) .Select(p => p.Id) .Concat(visibleUserIds) @@ -907,7 +907,7 @@ public class NoteService( { try { - return await userResolver.ResolveAsync(p.Href!.Id!); + return await userResolver.ResolveAsync(p.Href!.Id!, true); } catch { @@ -930,7 +930,7 @@ public class NoteService( { try { - return await userResolver.ResolveAsync(p.Acct); + return await userResolver.ResolveAsync(p.Acct, true); } catch { @@ -1096,7 +1096,7 @@ public class NoteService( if (res != null && !forceRefresh) return res; } - var actor = await userResolver.ResolveAsync(attrTo.Id); + var actor = await userResolver.ResolveAsync(attrTo.Id, true); using (await KeyedLocker.LockAsync(uri)) { diff --git a/Iceshrimp.Backend/Core/Services/UserProfileMentionsResolver.cs b/Iceshrimp.Backend/Core/Services/UserProfileMentionsResolver.cs index 9b0b92f6..63b7fcd0 100644 --- a/Iceshrimp.Backend/Core/Services/UserProfileMentionsResolver.cs +++ b/Iceshrimp.Backend/Core/Services/UserProfileMentionsResolver.cs @@ -39,12 +39,12 @@ public class UserProfileMentionsResolver(ActivityPub.UserResolver userResolver, var users = await mentionNodes .DistinctBy(p => p.Acct) - .Select(async p => await userResolver.ResolveAsyncOrNull(p.Username, p.Host?.Value ?? host)) + .Select(p => userResolver.ResolveAsyncOrNull(p.Username, p.Host?.Value ?? host, true)) .AwaitAllNoConcurrencyAsync(); users.AddRange(await userUris .Distinct() - .Select(async p => await userResolver.ResolveAsyncOrNull(p)) + .Select(p => userResolver.ResolveAsyncOrNull(p, true)) .AwaitAllNoConcurrencyAsync()); var mentions = users.Where(p => p != null) @@ -78,11 +78,11 @@ public class UserProfileMentionsResolver(ActivityPub.UserResolver userResolver, .Cast() .ToList(); - var nodes = input.SelectMany(p => MfmParser.Parse(p)); + var nodes = input.SelectMany(MfmParser.Parse); var mentionNodes = EnumerateMentions(nodes); var users = await mentionNodes .DistinctBy(p => p.Acct) - .Select(async p => await userResolver.ResolveAsyncOrNull(p.Username, p.Host?.Value ?? host)) + .Select(p => userResolver.ResolveAsyncOrNull(p.Username, p.Host?.Value ?? host, true)) .AwaitAllNoConcurrencyAsync(); return users.Where(p => p != null)