From a3ad95c058c02fa94d3f72ce49f5068d4215a436 Mon Sep 17 00:00:00 2001 From: Julia Date: Wed, 20 Nov 2024 18:24:50 -0500 Subject: [PATCH] Merge commit from fork * Fix poll update spoofing * fix: Disallow negative poll counts --------- Co-authored-by: syuilo <4439005+syuilo@users.noreply.github.com> Signed-off-by: eternal-flame-AD --- .../src/core/activitypub/ApInboxService.ts | 2 +- .../activitypub/models/ApQuestionService.ts | 29 ++++++++++++++----- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/packages/backend/src/core/activitypub/ApInboxService.ts b/packages/backend/src/core/activitypub/ApInboxService.ts index 64ddd76bfa..9d0d8f486c 100644 --- a/packages/backend/src/core/activitypub/ApInboxService.ts +++ b/packages/backend/src/core/activitypub/ApInboxService.ts @@ -835,7 +835,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, resolver).catch(err => console.error(err)); + await this.apQuestionService.updateQuestion(object, actor, resolver).catch(err => console.error(err)); return 'ok: Question updated'; } else { return `skip: Unknown type: ${getApType(object)}`; diff --git a/packages/backend/src/core/activitypub/models/ApQuestionService.ts b/packages/backend/src/core/activitypub/models/ApQuestionService.ts index 7a040aa7e8..33fbbd54c3 100644 --- a/packages/backend/src/core/activitypub/models/ApQuestionService.ts +++ b/packages/backend/src/core/activitypub/models/ApQuestionService.ts @@ -5,16 +5,17 @@ import { Inject, Injectable } from '@nestjs/common'; import { DI } from '@/di-symbols.js'; -import type { NotesRepository, PollsRepository } from '@/models/_.js'; +import type { UsersRepository, NotesRepository, PollsRepository } from '@/models/_.js'; import type { Config } from '@/config.js'; import type { IPoll } from '@/models/Poll.js'; +import type { MiRemoteUser } from '@/models/User.js'; import type Logger from '@/logger.js'; import { bindThis } from '@/decorators.js'; -import { isQuestion } from '../type.js'; +import { getOneApId, isQuestion } from '../type.js'; import { ApLoggerService } from '../ApLoggerService.js'; import { ApResolverService } from '../ApResolverService.js'; import type { Resolver } from '../ApResolverService.js'; -import type { IObject, IQuestion } from '../type.js'; +import type { IObject } from '../type.js'; import { yumeAssertAcceptableURL } from '../misc/validator.js'; import { toASCII } from 'punycode'; @@ -26,6 +27,9 @@ export class ApQuestionService { @Inject(DI.config) private config: Config, + @Inject(DI.usersRepository) + private usersRepository: UsersRepository, + @Inject(DI.notesRepository) private notesRepository: NotesRepository, @@ -67,7 +71,7 @@ export class ApQuestionService { * @returns true if updated */ @bindThis - public async updateQuestion(value: string | IObject, resolver?: Resolver): Promise { + public async updateQuestion(value: string | IObject, actor?: MiRemoteUser, resolver?: Resolver): Promise { const uriIn = typeof value === 'string' ? value : value.id; if (uriIn == null) throw new Error('uri is null'); @@ -82,15 +86,26 @@ export class ApQuestionService { const poll = await this.pollsRepository.findOneBy({ noteId: note.id }); if (poll == null) throw new Error('Question is not registered'); + + const user = await this.usersRepository.findOneBy({ id: poll.userId }); + if (user == null) throw new Error('Question is not registered'); //#endregion // resolve new Question object // eslint-disable-next-line no-param-reassign if (resolver == null) resolver = this.apResolverService.createResolver(); - const question = await resolver.resolve(value) as IQuestion; + const question = await resolver.resolve(value); this.logger.debug(`fetched question: ${JSON.stringify(question, null, 2)}`); - if (question.type !== 'Question') throw new Error('object is not a Question'); + if (!isQuestion(question)) throw new Error('object is not a Question'); + + const attribution = (question.attributedTo) ? getOneApId(question.attributedTo) : user.uri; + const attributionMatchesExisting = attribution === user.uri; + const actorMatchesAttribution = (actor) ? attribution === actor.uri : true; + + if (!attributionMatchesExisting || !actorMatchesAttribution) { + throw new Error('Refusing to ingest update for poll by different user'); + } const apChoices = question.oneOf ?? question.anyOf; if (apChoices == null) throw new Error('invalid apChoices: ' + apChoices); @@ -100,7 +115,7 @@ export class ApQuestionService { for (const choice of poll.choices) { const oldCount = poll.votes[poll.choices.indexOf(choice)]; const newCount = apChoices.filter(ap => ap.name === choice).at(0)?.replies?.totalItems; - if (newCount == null) throw new Error('invalid newCount: ' + newCount); + if (newCount == null || !(Number.isInteger(newCount) && newCount >= 0)) throw new Error('invalid newCount: ' + newCount); if (oldCount !== newCount) { changed = true;