From 6a775127379c5534fd3e0f664f5039d77717ab59 Mon Sep 17 00:00:00 2001 From: piuvas Date: Sat, 19 Apr 2025 23:04:48 -0300 Subject: [PATCH 1/8] refactor link verification. --- .../backend/src/misc/verify-field-link.ts | 32 +++++++++++++++++++ .../src/server/api/endpoints/i/update.ts | 13 +++++++- 2 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 packages/backend/src/misc/verify-field-link.ts diff --git a/packages/backend/src/misc/verify-field-link.ts b/packages/backend/src/misc/verify-field-link.ts new file mode 100644 index 0000000000..9f9a37d655 --- /dev/null +++ b/packages/backend/src/misc/verify-field-link.ts @@ -0,0 +1,32 @@ +/* +* SPDX-FileCopyrightText: piuvas and other Sharkey contributors +* SPDX-License-Identifier: AGPL-3.0-only +*/ + +import { JSDOM } from 'jsdom'; +import { HttpRequestService } from '@/core/HttpRequestService.js'; +import { safeForSql } from './safe-for-sql.js'; + + +export async function verifyFieldLink(field_url: string, profile_url: string, httpRequestService: HttpRequestService): Promise { + if (!safeForSql(field_url)) return; + + try { + const html = await httpRequestService.getHtml(field_url); + + const { window } = new JSDOM(html); + const doc: Document = window.document; + + const aEls = Array.from(doc.getElementsByTagName('a')); + const linkEls = Array.from(doc.getElementsByTagName('link')); + + const includesProfileLinks = [...aEls, ...linkEls].some(link => link.rel === 'me' && link.href === profile_url); + + window.close(); + + return includesProfileLinks; + } catch (err) { + // なにもしない + return; + } +} diff --git a/packages/backend/src/server/api/endpoints/i/update.ts b/packages/backend/src/server/api/endpoints/i/update.ts index f1d201d081..b6675505e0 100644 --- a/packages/backend/src/server/api/endpoints/i/update.ts +++ b/packages/backend/src/server/api/endpoints/i/update.ts @@ -31,6 +31,7 @@ import { DriveFileEntityService } from '@/core/entities/DriveFileEntityService.j import { HttpRequestService } from '@/core/HttpRequestService.js'; import type { Config } from '@/config.js'; import { safeForSql } from '@/misc/safe-for-sql.js'; +import { verifyFieldLink } from '@/misc/verify-field-link.js' import { AvatarDecorationService } from '@/core/AvatarDecorationService.js'; import { notificationRecieveConfig } from '@/models/json-schema/user.js'; import { userUnsignedFetchOptions } from '@/const.js'; @@ -613,13 +614,23 @@ export default class extends Endpoint { // eslint- const urls = updatedProfile.fields.filter(x => x.value.startsWith('https://')); for (const url of urls) { - this.verifyLink(url.value, user); + // this is a different, broader implementation so we can support remote users. + const includesProfileLinks = await verifyFieldLink(url.value, `${this.config.url}/@${user.username}`, this.httpRequestService); + if (includesProfileLinks) { + await userProfilesRepository.createQueryBuilder('profile').update() + .where('userId = :userId', { userId: user.id }) + .set({ + verifiedLinks: () => `array_append("verifiedLinks", '${url}')`, // ここでSQLインジェクションされそうなのでとりあえず safeForSql で弾いている + }) + .execute(); + } } return iObj; }); } + // this function is superseded by '@/misc/verify-field-link.ts' private async verifyLink(url: string, user: MiLocalUser) { if (!safeForSql(url)) return; From 8a60c7df0204d71eecbd4fce88c92cf39fa174c2 Mon Sep 17 00:00:00 2001 From: piuvas Date: Sat, 19 Apr 2025 23:10:27 -0300 Subject: [PATCH 2/8] verify links in remote profiles. --- .../activitypub/models/ApPersonService.ts | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/packages/backend/src/core/activitypub/models/ApPersonService.ts b/packages/backend/src/core/activitypub/models/ApPersonService.ts index d624632612..32889a3630 100644 --- a/packages/backend/src/core/activitypub/models/ApPersonService.ts +++ b/packages/backend/src/core/activitypub/models/ApPersonService.ts @@ -41,6 +41,8 @@ import { DriveFileEntityService } from '@/core/entities/DriveFileEntityService.j import type { AccountMoveService } from '@/core/AccountMoveService.js'; import { ApUtilityService } from '@/core/activitypub/ApUtilityService.js'; import { MemoryKVCache } from '@/misc/cache.js'; +import { HttpRequestService } from '@/core/HttpRequestService.js'; +import { verifyFieldLink } from '@/misc/verify-field-link.js'; import { getApId, getApType, isActor, isCollection, isCollectionOrOrderedCollection, isPropertyValue } from '../type.js'; import { extractApHashtags } from './tag.js'; import type { OnModuleInit } from '@nestjs/common'; @@ -112,6 +114,7 @@ export class ApPersonService implements OnModuleInit, OnApplicationShutdown { private roleService: RoleService, private readonly apUtilityService: ApUtilityService, + private httpRequestService: HttpRequestService, ) { } @@ -334,6 +337,7 @@ export class ApPersonService implements OnModuleInit, OnApplicationShutdown { this.logger.info(`Creating the Person: ${person.id}`); const fields = this.analyzeAttachments(person.attachment ?? []); + const field_urls = fields.filter(x => x.value.includes('https://')); const tags = extractApHashtags(person.tag).map(normalizeForSearch).splice(0, 32); @@ -362,6 +366,16 @@ export class ApPersonService implements OnModuleInit, OnApplicationShutdown { const url = this.apUtilityService.findBestObjectUrl(person); + const verifiedLinks: string[] = []; + if (url) { + for (const field_url of field_urls) { + const includesProfileLinks = await verifyFieldLink(field_url.value, url, this.httpRequestService); + if (includesProfileLinks) { + verifiedLinks.push(field_url.value) + } + } + } + // Create user let user: MiRemoteUser | null = null; let publicKey: MiUserPublickey | null = null; @@ -436,6 +450,7 @@ export class ApPersonService implements OnModuleInit, OnApplicationShutdown { followedMessage: person._misskey_followedMessage != null ? truncate(person._misskey_followedMessage, 256) : null, url, fields, + verifiedLinks, followingVisibility, followersVisibility, birthday: bday?.[0] ?? null, @@ -551,6 +566,7 @@ export class ApPersonService implements OnModuleInit, OnApplicationShutdown { const emojiNames = emojis.map(emoji => emoji.name); const fields = this.analyzeAttachments(person.attachment ?? []); + const field_urls = fields.filter(x => x.value.includes('https://')); const tags = extractApHashtags(person.tag).map(normalizeForSearch).splice(0, 32); @@ -579,6 +595,16 @@ export class ApPersonService implements OnModuleInit, OnApplicationShutdown { const url = this.apUtilityService.findBestObjectUrl(person); + const verifiedLinks: string[] = []; + if (url) { + for (const field_url of field_urls) { + const includesProfileLinks = await verifyFieldLink(field_url.value, url, this.httpRequestService); + if (includesProfileLinks) { + verifiedLinks.push(field_url.value) + } + } + } + const updates = { lastFetchedAt: new Date(), inbox: person.inbox, @@ -661,6 +687,7 @@ export class ApPersonService implements OnModuleInit, OnApplicationShutdown { await this.userProfilesRepository.update({ userId: exist.id }, { url, fields, + verifiedLinks, description: _description, followedMessage: person._misskey_followedMessage != null ? truncate(person._misskey_followedMessage, 256) : null, followingVisibility, From 1d9876d3fa9e0332720909b1f773d6c18cde800a Mon Sep 17 00:00:00 2001 From: piuvas Date: Sat, 19 Apr 2025 23:20:21 -0300 Subject: [PATCH 3/8] make link detection slightly more performant. --- .../backend/src/core/activitypub/models/ApPersonService.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/backend/src/core/activitypub/models/ApPersonService.ts b/packages/backend/src/core/activitypub/models/ApPersonService.ts index 32889a3630..ad65be53a7 100644 --- a/packages/backend/src/core/activitypub/models/ApPersonService.ts +++ b/packages/backend/src/core/activitypub/models/ApPersonService.ts @@ -337,7 +337,7 @@ export class ApPersonService implements OnModuleInit, OnApplicationShutdown { this.logger.info(`Creating the Person: ${person.id}`); const fields = this.analyzeAttachments(person.attachment ?? []); - const field_urls = fields.filter(x => x.value.includes('https://')); + const field_urls = fields.filter(x => x.value.startsWith('https://')); const tags = extractApHashtags(person.tag).map(normalizeForSearch).splice(0, 32); @@ -566,7 +566,7 @@ export class ApPersonService implements OnModuleInit, OnApplicationShutdown { const emojiNames = emojis.map(emoji => emoji.name); const fields = this.analyzeAttachments(person.attachment ?? []); - const field_urls = fields.filter(x => x.value.includes('https://')); + const field_urls = fields.filter(x => x.value.startsWith('https://')); const tags = extractApHashtags(person.tag).map(normalizeForSearch).splice(0, 32); From 20482888b06e9821d293a02c2f7a5021171c1127 Mon Sep 17 00:00:00 2001 From: piuvas Date: Sun, 20 Apr 2025 10:44:40 -0300 Subject: [PATCH 4/8] add merge guide for verifyLink. --- CONTRIBUTING.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4c0b370146..abbbdcded7 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -680,6 +680,7 @@ seems to do a decent job) * from `packages/backend/src/queue/processors/InboxProcessorService.ts` to `packages/backend/src/core/UpdateInstanceQueue.ts` where `updateInstanceQueue` is impacted + * from `verifyLink` in `packages/backend/src/core/activitypub/models/ApPersonService.ts` to `verifyFieldLink` in `packages/backend/src/misc/verify-field-link.ts` (if sensible) * if there have been any changes to the federated user data (the `renderPerson` function in `packages/backend/src/core/activitypub/ApRendererService.ts`), make From 46fa99fc282dc57a4f60c37080cc8a32a89c5492 Mon Sep 17 00:00:00 2001 From: piuvas Date: Sun, 20 Apr 2025 12:34:00 -0300 Subject: [PATCH 5/8] requested changes to verifyFieldLinks Co-authored-by: dakkar --- .../activitypub/models/ApPersonService.ts | 24 ++---------- .../backend/src/misc/verify-field-link.ts | 37 ++++++++++--------- .../src/server/api/endpoints/i/update.ts | 25 ++++++------- 3 files changed, 34 insertions(+), 52 deletions(-) diff --git a/packages/backend/src/core/activitypub/models/ApPersonService.ts b/packages/backend/src/core/activitypub/models/ApPersonService.ts index ad65be53a7..fcc1f58bbc 100644 --- a/packages/backend/src/core/activitypub/models/ApPersonService.ts +++ b/packages/backend/src/core/activitypub/models/ApPersonService.ts @@ -42,7 +42,7 @@ import type { AccountMoveService } from '@/core/AccountMoveService.js'; import { ApUtilityService } from '@/core/activitypub/ApUtilityService.js'; import { MemoryKVCache } from '@/misc/cache.js'; import { HttpRequestService } from '@/core/HttpRequestService.js'; -import { verifyFieldLink } from '@/misc/verify-field-link.js'; +import { verifyFieldLinks } from '@/misc/verify-field-link.js'; import { getApId, getApType, isActor, isCollection, isCollectionOrOrderedCollection, isPropertyValue } from '../type.js'; import { extractApHashtags } from './tag.js'; import type { OnModuleInit } from '@nestjs/common'; @@ -337,7 +337,6 @@ export class ApPersonService implements OnModuleInit, OnApplicationShutdown { this.logger.info(`Creating the Person: ${person.id}`); const fields = this.analyzeAttachments(person.attachment ?? []); - const field_urls = fields.filter(x => x.value.startsWith('https://')); const tags = extractApHashtags(person.tag).map(normalizeForSearch).splice(0, 32); @@ -366,15 +365,7 @@ export class ApPersonService implements OnModuleInit, OnApplicationShutdown { const url = this.apUtilityService.findBestObjectUrl(person); - const verifiedLinks: string[] = []; - if (url) { - for (const field_url of field_urls) { - const includesProfileLinks = await verifyFieldLink(field_url.value, url, this.httpRequestService); - if (includesProfileLinks) { - verifiedLinks.push(field_url.value) - } - } - } + const verifiedLinks = url ? await verifyFieldLinks(fields, url, this.httpRequestService) : []; // Create user let user: MiRemoteUser | null = null; @@ -566,7 +557,6 @@ export class ApPersonService implements OnModuleInit, OnApplicationShutdown { const emojiNames = emojis.map(emoji => emoji.name); const fields = this.analyzeAttachments(person.attachment ?? []); - const field_urls = fields.filter(x => x.value.startsWith('https://')); const tags = extractApHashtags(person.tag).map(normalizeForSearch).splice(0, 32); @@ -595,15 +585,7 @@ export class ApPersonService implements OnModuleInit, OnApplicationShutdown { const url = this.apUtilityService.findBestObjectUrl(person); - const verifiedLinks: string[] = []; - if (url) { - for (const field_url of field_urls) { - const includesProfileLinks = await verifyFieldLink(field_url.value, url, this.httpRequestService); - if (includesProfileLinks) { - verifiedLinks.push(field_url.value) - } - } - } + const verifiedLinks = url ? await verifyFieldLinks(fields, url, this.httpRequestService) : []; const updates = { lastFetchedAt: new Date(), diff --git a/packages/backend/src/misc/verify-field-link.ts b/packages/backend/src/misc/verify-field-link.ts index 9f9a37d655..1207d25eee 100644 --- a/packages/backend/src/misc/verify-field-link.ts +++ b/packages/backend/src/misc/verify-field-link.ts @@ -5,28 +5,31 @@ import { JSDOM } from 'jsdom'; import { HttpRequestService } from '@/core/HttpRequestService.js'; -import { safeForSql } from './safe-for-sql.js'; +type Field = { name: string, value: string }; -export async function verifyFieldLink(field_url: string, profile_url: string, httpRequestService: HttpRequestService): Promise { - if (!safeForSql(field_url)) return; +export async function verifyFieldLinks(fields: Field[], profile_url: string, httpRequestService: HttpRequestService): Promise { + const verified_links = []; + for (const field_url of fields + .filter(x => URL.canParse(x.value) && ['http:', 'https:'].includes((new URL(x.value).protocol)))) { + console.log('fortnite ' + field_url); + try { + const html = await httpRequestService.getHtml(field_url.value); - try { - const html = await httpRequestService.getHtml(field_url); + const { window } = new JSDOM(html); + const doc: Document = window.document; - const { window } = new JSDOM(html); - const doc: Document = window.document; + const aEls = Array.from(doc.getElementsByTagName('a')); + const linkEls = Array.from(doc.getElementsByTagName('link')); - const aEls = Array.from(doc.getElementsByTagName('a')); - const linkEls = Array.from(doc.getElementsByTagName('link')); + const includesProfileLinks = [...aEls, ...linkEls].some(link => link.rel === 'me' && link.href === profile_url); + if (includesProfileLinks) { verified_links.push(field_url.value); } - const includesProfileLinks = [...aEls, ...linkEls].some(link => link.rel === 'me' && link.href === profile_url); - - window.close(); - - return includesProfileLinks; - } catch (err) { - // なにもしない - return; + window.close(); + } catch (err) { + // don't do anything. + continue; + } } + return verified_links; } diff --git a/packages/backend/src/server/api/endpoints/i/update.ts b/packages/backend/src/server/api/endpoints/i/update.ts index b6675505e0..f8937a8919 100644 --- a/packages/backend/src/server/api/endpoints/i/update.ts +++ b/packages/backend/src/server/api/endpoints/i/update.ts @@ -31,7 +31,7 @@ import { DriveFileEntityService } from '@/core/entities/DriveFileEntityService.j import { HttpRequestService } from '@/core/HttpRequestService.js'; import type { Config } from '@/config.js'; import { safeForSql } from '@/misc/safe-for-sql.js'; -import { verifyFieldLink } from '@/misc/verify-field-link.js' +import { verifyFieldLinks } from '@/misc/verify-field-link.js'; import { AvatarDecorationService } from '@/core/AvatarDecorationService.js'; import { notificationRecieveConfig } from '@/models/json-schema/user.js'; import { userUnsignedFetchOptions } from '@/const.js'; @@ -585,9 +585,11 @@ export default class extends Endpoint { // eslint- this.globalEventService.publishInternalEvent('localUserUpdated', { id: user.id }); } + const verified_links = await verifyFieldLinks(newFields, `${this.config.url}/@${user.username}`, this.httpRequestService); + await this.userProfilesRepository.update(user.id, { ...profileUpdates, - verifiedLinks: [], + verifiedLinks: verified_links, }); const iObj = await this.userEntityService.pack(user.id, user, { @@ -612,18 +614,13 @@ export default class extends Endpoint { // eslint- this.accountUpdateService.publishToFollowers(user.id); } - const urls = updatedProfile.fields.filter(x => x.value.startsWith('https://')); - for (const url of urls) { - // this is a different, broader implementation so we can support remote users. - const includesProfileLinks = await verifyFieldLink(url.value, `${this.config.url}/@${user.username}`, this.httpRequestService); - if (includesProfileLinks) { - await userProfilesRepository.createQueryBuilder('profile').update() - .where('userId = :userId', { userId: user.id }) - .set({ - verifiedLinks: () => `array_append("verifiedLinks", '${url}')`, // ここでSQLインジェクションされそうなのでとりあえず safeForSql で弾いている - }) - .execute(); - } + if (verified_links.length > 0) { + await userProfilesRepository.createQueryBuilder('profile').update() + .where('userId = :userId', { userId: user.id }) + .set({ + verifiedLinks: verified_links.filter(x => safeForSql(x)), // ここでSQLインジェクションされそうなのでとりあえず safeForSql で弾いている + }) + .execute(); } return iObj; From 8609426e7136ccfb8b6d4eb42702656c771ba4ba Mon Sep 17 00:00:00 2001 From: piuvas Date: Sun, 20 Apr 2025 14:21:44 -0300 Subject: [PATCH 6/8] remove fortnite. --- packages/backend/src/misc/verify-field-link.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/backend/src/misc/verify-field-link.ts b/packages/backend/src/misc/verify-field-link.ts index 1207d25eee..65294158c4 100644 --- a/packages/backend/src/misc/verify-field-link.ts +++ b/packages/backend/src/misc/verify-field-link.ts @@ -12,7 +12,6 @@ export async function verifyFieldLinks(fields: Field[], profile_url: string, htt const verified_links = []; for (const field_url of fields .filter(x => URL.canParse(x.value) && ['http:', 'https:'].includes((new URL(x.value).protocol)))) { - console.log('fortnite ' + field_url); try { const html = await httpRequestService.getHtml(field_url.value); From 06fb6fbecad744914837f1f8b6844088076631e4 Mon Sep 17 00:00:00 2001 From: piuvas Date: Sun, 20 Apr 2025 23:20:59 -0300 Subject: [PATCH 7/8] requested changes. --- packages/backend/src/misc/verify-field-link.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend/src/misc/verify-field-link.ts b/packages/backend/src/misc/verify-field-link.ts index 65294158c4..f519acfba0 100644 --- a/packages/backend/src/misc/verify-field-link.ts +++ b/packages/backend/src/misc/verify-field-link.ts @@ -4,7 +4,7 @@ */ import { JSDOM } from 'jsdom'; -import { HttpRequestService } from '@/core/HttpRequestService.js'; +import type { HttpRequestService } from '@/core/HttpRequestService.js'; type Field = { name: string, value: string }; From 6df82f4eefc31e84fc311ff7fff5c59cc4108195 Mon Sep 17 00:00:00 2001 From: piuvas Date: Sun, 20 Apr 2025 23:21:50 -0300 Subject: [PATCH 8/8] remove redundant sql query. --- packages/backend/src/server/api/endpoints/i/update.ts | 9 --------- 1 file changed, 9 deletions(-) diff --git a/packages/backend/src/server/api/endpoints/i/update.ts b/packages/backend/src/server/api/endpoints/i/update.ts index f8937a8919..3566941957 100644 --- a/packages/backend/src/server/api/endpoints/i/update.ts +++ b/packages/backend/src/server/api/endpoints/i/update.ts @@ -614,15 +614,6 @@ export default class extends Endpoint { // eslint- this.accountUpdateService.publishToFollowers(user.id); } - if (verified_links.length > 0) { - await userProfilesRepository.createQueryBuilder('profile').update() - .where('userId = :userId', { userId: user.id }) - .set({ - verifiedLinks: verified_links.filter(x => safeForSql(x)), // ここでSQLインジェクションされそうなのでとりあえず safeForSql で弾いている - }) - .execute(); - } - return iObj; }); }