Merge commit from fork

* Fix poll update spoofing

* fix: Disallow negative poll counts

---------

Co-authored-by: syuilo <4439005+syuilo@users.noreply.github.com>
(cherry picked from commit b9cb949eb1f8c57eaa98fc5446d902cf8a5ea85c)
This commit is contained in:
Julia 2024-11-20 18:24:50 -05:00 committed by あわわわとーにゅ
parent 85096e58b9
commit f1b5708971
No known key found for this signature in database
GPG key ID: 6AFBBF529601C1DB
3 changed files with 25 additions and 10 deletions

View file

@ -804,7 +804,7 @@ export class ApInboxService {
await this.apPersonService.updatePerson(actor.uri, resolver, object); await this.apPersonService.updatePerson(actor.uri, resolver, object);
return 'ok: Person updated'; return 'ok: Person updated';
} else if (getApType(object) === 'Question') { } else if (getApType(object) === 'Question') {
await this.apQuestionService.updateQuestion(object, resolver).catch(err => this.logger.error(`err: failed to update question: ${err}`, { error: err })); await this.apQuestionService.updateQuestion(object, actor, resolver).catch(err => this.logger.error(`err: failed to update question: ${err}`, { error: err }));
return 'ok: Question updated'; return 'ok: Question updated';
} else if (additionalCc && isPost(object)) { } else if (additionalCc && isPost(object)) {
const uri = getApId(object); const uri = getApId(object);

View file

@ -5,18 +5,19 @@
import { Inject, Injectable } from '@nestjs/common'; import { Inject, Injectable } from '@nestjs/common';
import { DI } from '@/di-symbols.js'; 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 { Config } from '@/config.js';
import type { IPoll } from '@/models/Poll.js'; import type { IPoll } from '@/models/Poll.js';
import type { MiRemoteUser } from '@/models/User.js';
import type Logger from '@/logger.js'; import type Logger from '@/logger.js';
import { bindThis } from '@/decorators.js'; import { bindThis } from '@/decorators.js';
import { isNotNull } from '@/misc/is-not-null.js'; import { isNotNull } from '@/misc/is-not-null.js';
import { UtilityService } from '@/core/UtilityService.js'; import { UtilityService } from '@/core/UtilityService.js';
import { isQuestion } from '../type.js'; import { getOneApId, isQuestion } from '../type.js';
import { ApLoggerService } from '../ApLoggerService.js'; import { ApLoggerService } from '../ApLoggerService.js';
import { ApResolverService } from '../ApResolverService.js'; import { ApResolverService } from '../ApResolverService.js';
import type { Resolver } from '../ApResolverService.js'; import type { Resolver } from '../ApResolverService.js';
import type { IObject, IQuestion } from '../type.js'; import type { IObject } from '../type.js';
@Injectable() @Injectable()
export class ApQuestionService { export class ApQuestionService {
@ -26,6 +27,9 @@ export class ApQuestionService {
@Inject(DI.config) @Inject(DI.config)
private config: Config, private config: Config,
@Inject(DI.usersRepository)
private usersRepository: UsersRepository,
@Inject(DI.notesRepository) @Inject(DI.notesRepository)
private notesRepository: NotesRepository, private notesRepository: NotesRepository,
@ -68,7 +72,7 @@ export class ApQuestionService {
* @returns true if updated * @returns true if updated
*/ */
@bindThis @bindThis
public async updateQuestion(value: string | IObject, resolver?: Resolver): Promise<boolean> { public async updateQuestion(value: string | IObject, actor?: MiRemoteUser, resolver?: Resolver): Promise<boolean> {
const uri = typeof value === 'string' ? value : value.id; const uri = typeof value === 'string' ? value : value.id;
if (uri == null) throw new Error('uri is null'); if (uri == null) throw new Error('uri is null');
@ -80,16 +84,27 @@ export class ApQuestionService {
if (note == null) throw new Error('Question is not registed'); if (note == null) throw new Error('Question is not registed');
const poll = await this.pollsRepository.findOneBy({ noteId: note.id }); const poll = await this.pollsRepository.findOneBy({ noteId: note.id });
if (poll == null) throw new Error('Question is not registed'); 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('User is not registered');
//#endregion //#endregion
// resolve new Question object // resolve new Question object
// eslint-disable-next-line no-param-reassign // eslint-disable-next-line no-param-reassign
if (resolver == null) resolver = this.apResolverService.createResolver(); 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)}`); 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; const apChoices = question.oneOf ?? question.anyOf;
if (apChoices == null) throw new Error('invalid apChoices: ' + apChoices); if (apChoices == null) throw new Error('invalid apChoices: ' + apChoices);
@ -99,7 +114,7 @@ export class ApQuestionService {
for (const choice of poll.choices) { for (const choice of poll.choices) {
const oldCount = poll.votes[poll.choices.indexOf(choice)]; const oldCount = poll.votes[poll.choices.indexOf(choice)];
const newCount = apChoices.filter(ap => ap.name === choice).at(0)?.replies?.totalItems; 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) { if (oldCount !== newCount) {
changed = true; changed = true;

View file

@ -200,7 +200,7 @@ export class InboxProcessorService implements OnApplicationShutdown {
// アクティビティを処理 // アクティビティを処理
try { try {
const result = await this.apInboxService.performActivity(authUser.user, activity, job.data.user?.id); const result = await this.apInboxService.performActivity(authUser.user, activity, undefined, job.data.user?.id);
if (result && !result.startsWith('ok')) { if (result && !result.startsWith('ok')) {
this.logger.warn(`inbox activity ignored (maybe): id=${activity.id} reason=${result}`); this.logger.warn(`inbox activity ignored (maybe): id=${activity.id} reason=${result}`);
return result; return result;