From 12ba460e9ebf4951f9c1caee8822a8ca1523563f Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Wed, 19 Sep 2018 15:47:55 +0200 Subject: [PATCH] Improve AP actor checks --- server/lib/activitypub/cache-file.ts | 4 +++ .../lib/activitypub/process/process-delete.ts | 4 +++ .../lib/activitypub/process/process-reject.ts | 6 ++-- .../lib/activitypub/process/process-undo.ts | 32 ++++++++----------- server/lib/activitypub/process/process.ts | 5 +++ 5 files changed, 30 insertions(+), 21 deletions(-) diff --git a/server/lib/activitypub/cache-file.ts b/server/lib/activitypub/cache-file.ts index 20558daf9..87f8a4162 100644 --- a/server/lib/activitypub/cache-file.ts +++ b/server/lib/activitypub/cache-file.ts @@ -31,6 +31,10 @@ function createCacheFile (cacheFileObject: CacheFileObject, video: VideoModel, b } function updateCacheFile (cacheFileObject: CacheFileObject, redundancyModel: VideoRedundancyModel, byActor: { id?: number }) { + if (redundancyModel.actorId !== byActor.id) { + throw new Error('Cannot update redundancy ' + redundancyModel.url + ' of another actor.') + } + const attributes = cacheFileActivityObjectToDBAttributes(cacheFileObject, redundancyModel.VideoFile.Video, byActor) redundancyModel.set('expires', attributes.expiresOn) diff --git a/server/lib/activitypub/process/process-delete.ts b/server/lib/activitypub/process/process-delete.ts index bf2a4d114..038d8c4d3 100644 --- a/server/lib/activitypub/process/process-delete.ts +++ b/server/lib/activitypub/process/process-delete.ts @@ -94,6 +94,10 @@ function processDeleteVideoComment (byActor: ActorModel, videoComment: VideoComm logger.debug('Removing remote video comment "%s".', videoComment.url) return sequelizeTypescript.transaction(async t => { + if (videoComment.Account.id !== byActor.Account.id) { + throw new Error('Account ' + byActor.url + ' does not own video comment ' + videoComment.url) + } + await videoComment.destroy({ transaction: t }) if (videoComment.Video.isOwned()) { diff --git a/server/lib/activitypub/process/process-reject.ts b/server/lib/activitypub/process/process-reject.ts index b0e678316..709a65096 100644 --- a/server/lib/activitypub/process/process-reject.ts +++ b/server/lib/activitypub/process/process-reject.ts @@ -17,11 +17,11 @@ export { // --------------------------------------------------------------------------- -async function processReject (actor: ActorModel, targetActor: ActorModel) { +async function processReject (follower: ActorModel, targetActor: ActorModel) { return sequelizeTypescript.transaction(async t => { - const actorFollow = await ActorFollowModel.loadByActorAndTarget(actor.id, targetActor.id, t) + const actorFollow = await ActorFollowModel.loadByActorAndTarget(follower.id, targetActor.id, t) - if (!actorFollow) throw new Error(`'Unknown actor follow ${actor.id} -> ${targetActor.id}.`) + if (!actorFollow) throw new Error(`'Unknown actor follow ${follower.id} -> ${targetActor.id}.`) await actorFollow.destroy({ transaction: t }) diff --git a/server/lib/activitypub/process/process-undo.ts b/server/lib/activitypub/process/process-undo.ts index c091d9678..73ca0a17c 100644 --- a/server/lib/activitypub/process/process-undo.ts +++ b/server/lib/activitypub/process/process-undo.ts @@ -1,10 +1,8 @@ import { ActivityAnnounce, ActivityFollow, ActivityLike, ActivityUndo, CacheFileObject } from '../../../../shared/models/activitypub' import { DislikeObject } from '../../../../shared/models/activitypub/objects' -import { getActorUrl } from '../../../helpers/activitypub' import { retryTransactionWrapper } from '../../../helpers/database-utils' import { logger } from '../../../helpers/logger' import { sequelizeTypescript } from '../../../initializers' -import { AccountModel } from '../../../models/account/account' import { AccountVideoRateModel } from '../../../models/account/account-video-rate' import { ActorModel } from '../../../models/activitypub/actor' import { ActorFollowModel } from '../../../models/activitypub/actor-follow' @@ -16,15 +14,13 @@ import { VideoRedundancyModel } from '../../../models/redundancy/video-redundanc async function processUndoActivity (activity: ActivityUndo, byActor: ActorModel) { const activityToUndo = activity.object - const actorUrl = getActorUrl(activity.actor) - if (activityToUndo.type === 'Like') { - return retryTransactionWrapper(processUndoLike, actorUrl, activity) + return retryTransactionWrapper(processUndoLike, byActor, activity) } if (activityToUndo.type === 'Create') { if (activityToUndo.object.type === 'Dislike') { - return retryTransactionWrapper(processUndoDislike, actorUrl, activity) + return retryTransactionWrapper(processUndoDislike, byActor, activity) } else if (activityToUndo.object.type === 'CacheFile') { return retryTransactionWrapper(processUndoCacheFile, byActor, activity) } @@ -51,48 +47,46 @@ export { // --------------------------------------------------------------------------- -async function processUndoLike (actorUrl: string, activity: ActivityUndo) { +async function processUndoLike (byActor: ActorModel, activity: ActivityUndo) { const likeActivity = activity.object as ActivityLike const { video } = await getOrCreateVideoAndAccountAndChannel({ videoObject: likeActivity.object }) return sequelizeTypescript.transaction(async t => { - const byAccount = await AccountModel.loadByUrl(actorUrl, t) - if (!byAccount) throw new Error('Unknown account ' + actorUrl) + if (!byActor.Account) throw new Error('Unknown account ' + byActor.url) - const rate = await AccountVideoRateModel.load(byAccount.id, video.id, t) - if (!rate) throw new Error(`Unknown rate by account ${byAccount.id} for video ${video.id}.`) + const rate = await AccountVideoRateModel.load(byActor.Account.id, video.id, t) + if (!rate) throw new Error(`Unknown rate by account ${byActor.Account.id} for video ${video.id}.`) await rate.destroy({ transaction: t }) await video.decrement('likes', { transaction: t }) if (video.isOwned()) { // Don't resend the activity to the sender - const exceptions = [ byAccount.Actor ] + const exceptions = [ byActor ] await forwardVideoRelatedActivity(activity, t, exceptions, video) } }) } -async function processUndoDislike (actorUrl: string, activity: ActivityUndo) { +async function processUndoDislike (byActor: ActorModel, activity: ActivityUndo) { const dislike = activity.object.object as DislikeObject const { video } = await getOrCreateVideoAndAccountAndChannel({ videoObject: dislike.object }) return sequelizeTypescript.transaction(async t => { - const byAccount = await AccountModel.loadByUrl(actorUrl, t) - if (!byAccount) throw new Error('Unknown account ' + actorUrl) + if (!byActor.Account) throw new Error('Unknown account ' + byActor.url) - const rate = await AccountVideoRateModel.load(byAccount.id, video.id, t) - if (!rate) throw new Error(`Unknown rate by account ${byAccount.id} for video ${video.id}.`) + const rate = await AccountVideoRateModel.load(byActor.Account.id, video.id, t) + if (!rate) throw new Error(`Unknown rate by account ${byActor.Account.id} for video ${video.id}.`) await rate.destroy({ transaction: t }) await video.decrement('dislikes', { transaction: t }) if (video.isOwned()) { // Don't resend the activity to the sender - const exceptions = [ byAccount.Actor ] + const exceptions = [ byActor ] await forwardVideoRelatedActivity(activity, t, exceptions, video) } @@ -108,6 +102,8 @@ async function processUndoCacheFile (byActor: ActorModel, activity: ActivityUndo const cacheFile = await VideoRedundancyModel.loadByUrl(cacheFileObject.id) if (!cacheFile) throw new Error('Unknown video cache ' + cacheFile.url) + if (cacheFile.actorId !== byActor.id) throw new Error('Cannot delete redundancy ' + cacheFile.url + ' of another actor.') + await cacheFile.destroy() if (video.isOwned()) { diff --git a/server/lib/activitypub/process/process.ts b/server/lib/activitypub/process/process.ts index 35ad1696a..b263f1ea2 100644 --- a/server/lib/activitypub/process/process.ts +++ b/server/lib/activitypub/process/process.ts @@ -29,6 +29,11 @@ async function processActivities (activities: Activity[], signatureActor?: Actor const actorsCache: { [ url: string ]: ActorModel } = {} for (const activity of activities) { + if (!signatureActor && [ 'Create', 'Announce', 'Like' ].indexOf(activity.type) === -1) { + logger.error('Cannot process activity %s (type: %s) without the actor signature.', activity.id, activity.type) + continue + } + const actorUrl = getActorUrl(activity.actor) // When we fetch remote data, we don't have signature -- 2.41.0