summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authordakkar <dakkar@thenautilus.net>2025-07-28 19:28:01 +0000
committerdakkar <dakkar@thenautilus.net>2025-07-28 19:28:01 +0000
commitce3531ed96da39a4e62cb3651df0b8aee49b912f (patch)
tree4def24de3c615351ba6ab86390029b393a81fa2f
parentmerge: Fix Postgres / TypeORM errors *FOR STABLE* (!1192) (diff)
parentfix DI error in HttpRequestService.ts (diff)
downloadsharkey-ce3531ed96da39a4e62cb3651df0b8aee49b912f.tar.gz
sharkey-ce3531ed96da39a4e62cb3651df0b8aee49b912f.tar.bz2
sharkey-ce3531ed96da39a4e62cb3651df0b8aee49b912f.zip
merge: Improve URL validation *FOR STABLE* (!1191)
View MR for information: https://activitypub.software/TransFem-org/Sharkey/-/merge_requests/1191 Approved-by: Hazelnoot <acomputerdog@gmail.com>
-rw-r--r--packages/backend/src/boot/entry.ts52
-rw-r--r--packages/backend/src/core/CaptchaService.ts17
-rw-r--r--packages/backend/src/core/DownloadService.ts4
-rw-r--r--packages/backend/src/core/HttpRequestService.ts45
-rw-r--r--packages/backend/src/core/UtilityService.ts59
-rw-r--r--packages/backend/src/core/activitypub/ApRequestService.ts4
-rw-r--r--packages/backend/src/core/activitypub/ApUtilityService.ts123
-rw-r--r--packages/backend/src/core/activitypub/models/ApNoteService.ts2
-rw-r--r--packages/backend/src/core/activitypub/models/ApPersonService.ts111
-rw-r--r--packages/backend/src/core/activitypub/type.ts6
-rw-r--r--packages/backend/src/misc/captcha-error.ts18
-rw-r--r--packages/backend/src/misc/render-inline-error.ts2
-rw-r--r--packages/backend/src/misc/verify-field-link.ts3
-rw-r--r--packages/backend/test/unit/FetchInstanceMetadataService.ts2
-rw-r--r--packages/backend/test/unit/core/HttpRequestService.ts65
-rw-r--r--packages/backend/test/unit/core/activitypub/ApUtilityService.ts140
16 files changed, 481 insertions, 172 deletions
diff --git a/packages/backend/src/boot/entry.ts b/packages/backend/src/boot/entry.ts
index afb48e526c..bbe6a57383 100644
--- a/packages/backend/src/boot/entry.ts
+++ b/packages/backend/src/boot/entry.ts
@@ -55,44 +55,42 @@ async function main() {
// Display detail of unhandled promise rejection
if (!envOption.quiet) {
process.on('unhandledRejection', e => {
- try {
- logger.error('Unhandled rejection:', inspect(e));
- } catch {
- console.error('Unhandled rejection:', inspect(e));
- }
+ logger.error('Unhandled rejection:', inspect(e));
});
}
- // Display detail of uncaught exception
- process.on('uncaughtExceptionMonitor', ((err, origin) => {
- try {
- logger.error(`Uncaught exception (${origin}):`, err);
- } catch {
- console.error(`Uncaught exception (${origin}):`, err);
+ process.on('uncaughtException', (err) => {
+ // Workaround for https://github.com/node-fetch/node-fetch/issues/954
+ if (String(err).match(/^TypeError: .+ is an? url with embedded credentials.$/)) {
+ logger.debug('Suppressed node-fetch issue#954, but the current job may fail.');
+ return;
+ }
+
+ // Workaround for https://github.com/node-fetch/node-fetch/issues/1845
+ if (String(err) === 'TypeError: Cannot read properties of undefined (reading \'body\')') {
+ logger.debug('Suppressed node-fetch issue#1845, but the current job may fail.');
+ return;
}
- }));
+
+ // Throw all other errors to avoid inconsistent state.
+ // (per NodeJS docs, it's unsafe to suppress arbitrary errors in an uncaughtException handler.)
+ throw err;
+ });
+
+ // Display detail of uncaught exception
+ process.on('uncaughtExceptionMonitor', (err, origin) => {
+ logger.error(`Uncaught exception (${origin}):`, err);
+ });
// Dying away...
process.on('disconnect', () => {
- try {
- logger.warn('IPC channel disconnected! The process may soon die.');
- } catch {
- console.warn('IPC channel disconnected! The process may soon die.');
- }
+ logger.warn('IPC channel disconnected! The process may soon die.');
});
process.on('beforeExit', code => {
- try {
- logger.warn(`Event loop died! Process will exit with code ${code}.`);
- } catch {
- console.warn(`Event loop died! Process will exit with code ${code}.`);
- }
+ logger.warn(`Event loop died! Process will exit with code ${code}.`);
});
process.on('exit', code => {
- try {
- logger.info(`The process is going to exit with code ${code}`);
- } catch {
- console.info(`The process is going to exit with code ${code}`);
- }
+ logger.info(`The process is going to exit with code ${code}`);
});
//#endregion
diff --git a/packages/backend/src/core/CaptchaService.ts b/packages/backend/src/core/CaptchaService.ts
index c526a80aeb..020984a37f 100644
--- a/packages/backend/src/core/CaptchaService.ts
+++ b/packages/backend/src/core/CaptchaService.ts
@@ -9,7 +9,10 @@ import { bindThis } from '@/decorators.js';
import { MetaService } from '@/core/MetaService.js';
import { MiMeta } from '@/models/Meta.js';
import Logger from '@/logger.js';
-import { LoggerService } from './LoggerService.js';
+import { LoggerService } from '@/core/LoggerService.js';
+import { CaptchaError } from '@/misc/captcha-error.js';
+
+export { CaptchaError } from '@/misc/captcha-error.js';
export const supportedCaptchaProviders = ['none', 'hcaptcha', 'mcaptcha', 'recaptcha', 'turnstile', 'fc', 'testcaptcha'] as const;
export type CaptchaProvider = typeof supportedCaptchaProviders[number];
@@ -49,18 +52,6 @@ export type CaptchaSetting = {
}
};
-export class CaptchaError extends Error {
- public readonly code: CaptchaErrorCode;
- public readonly cause?: unknown;
-
- constructor(code: CaptchaErrorCode, message: string, cause?: unknown) {
- super(message, cause ? { cause } : undefined);
- this.code = code;
- this.cause = cause;
- this.name = 'CaptchaError';
- }
-}
-
export type CaptchaSaveSuccess = {
success: true;
};
diff --git a/packages/backend/src/core/DownloadService.ts b/packages/backend/src/core/DownloadService.ts
index cb5bdb6cb7..d1819f40a3 100644
--- a/packages/backend/src/core/DownloadService.ts
+++ b/packages/backend/src/core/DownloadService.ts
@@ -19,6 +19,7 @@ import type Logger from '@/logger.js';
import { bindThis } from '@/decorators.js';
import { renderInlineError } from '@/misc/render-inline-error.js';
+import { UtilityService } from '@/core/UtilityService.js';
@Injectable()
export class DownloadService {
@@ -30,6 +31,7 @@ export class DownloadService {
private httpRequestService: HttpRequestService,
private loggerService: LoggerService,
+ private readonly utilityService: UtilityService,
) {
this.logger = this.loggerService.getLogger('download');
}
@@ -38,6 +40,8 @@ export class DownloadService {
public async downloadUrl(url: string, path: string, options: { timeout?: number, operationTimeout?: number, maxSize?: number } = {} ): Promise<{
filename: string;
}> {
+ this.utilityService.assertUrl(url);
+
this.logger.debug(`Downloading ${chalk.cyan(url)} to ${chalk.cyanBright(path)} ...`);
const timeout = options.timeout ?? 30 * 1000;
diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts
index 151097095d..bd72fefe4f 100644
--- a/packages/backend/src/core/HttpRequestService.ts
+++ b/packages/backend/src/core/HttpRequestService.ts
@@ -17,9 +17,9 @@ import { StatusError } from '@/misc/status-error.js';
import { bindThis } from '@/decorators.js';
import { validateContentTypeSetAsActivityPub } from '@/core/activitypub/misc/validator.js';
import type { IObject, IObjectWithId } from '@/core/activitypub/type.js';
-import { ApUtilityService } from './activitypub/ApUtilityService.js';
+import { UtilityService } from '@/core/UtilityService.js';
+import { ApUtilityService } from '@/core/activitypub/ApUtilityService.js';
import type { Response } from 'node-fetch';
-import type { URL } from 'node:url';
import type { Socket } from 'node:net';
export type HttpRequestSendOptions = {
@@ -27,7 +27,27 @@ export type HttpRequestSendOptions = {
validators?: ((res: Response) => void)[];
};
-export function isPrivateIp(allowedPrivateNetworks: PrivateNetwork[] | undefined, ip: string, port?: number): boolean {
+export async function isPrivateUrl(url: URL, lookup: net.LookupFunction): Promise<boolean> {
+ const ip = await resolveIp(url, lookup);
+ return ip.range() !== 'unicast';
+}
+
+export async function resolveIp(url: URL, lookup: net.LookupFunction) {
+ if (ipaddr.isValid(url.hostname)) {
+ return ipaddr.parse(url.hostname);
+ }
+
+ const resolvedIp = await new Promise<string>((resolve, reject) => {
+ lookup(url.hostname, {}, (err, address) => {
+ if (err) reject(err);
+ else resolve(address as string);
+ });
+ });
+
+ return ipaddr.parse(resolvedIp);
+}
+
+export function isAllowedPrivateIp(allowedPrivateNetworks: PrivateNetwork[] | undefined, ip: string, port?: number): boolean {
const parsedIp = ipaddr.parse(ip);
for (const { cidr, ports } of allowedPrivateNetworks ?? []) {
@@ -44,7 +64,7 @@ export function isPrivateIp(allowedPrivateNetworks: PrivateNetwork[] | undefined
export function validateSocketConnect(allowedPrivateNetworks: PrivateNetwork[] | undefined, socket: Socket): void {
const address = socket.remoteAddress;
if (address && ipaddr.isValid(address)) {
- if (isPrivateIp(allowedPrivateNetworks, address, socket.remotePort)) {
+ if (isAllowedPrivateIp(allowedPrivateNetworks, address, socket.remotePort)) {
socket.destroy(new Error(`Blocked address: ${address}`));
}
}
@@ -128,10 +148,16 @@ export class HttpRequestService {
*/
public readonly httpsAgent: https.Agent;
+ /**
+ * Get shared DNS resolver
+ */
+ public readonly lookup: net.LookupFunction;
+
constructor(
@Inject(DI.config)
private config: Config,
private readonly apUtilityService: ApUtilityService,
+ private readonly utilityService: UtilityService,
) {
const cache = new CacheableLookup({
maxTtl: 3600, // 1hours
@@ -139,6 +165,8 @@ export class HttpRequestService {
lookup: false, // nativeのdns.lookupにfallbackしない
});
+ this.lookup = cache.lookup as unknown as net.LookupFunction;
+
const agentOption = {
keepAlive: true,
keepAliveMsecs: 30 * 1000,
@@ -236,8 +264,6 @@ export class HttpRequestService {
@bindThis
public async getActivityJson(url: string, isLocalAddressAllowed = false, allowAnonymous = false): Promise<IObjectWithId> {
- this.apUtilityService.assertApUrl(url);
-
const res = await this.send(url, {
method: 'GET',
headers: {
@@ -303,6 +329,7 @@ export class HttpRequestService {
timeout?: number,
size?: number,
isLocalAddressAllowed?: boolean,
+ allowHttp?: boolean,
} = {},
extra: HttpRequestSendOptions = {
throwErrorWhenResponseNotOk: true,
@@ -311,6 +338,10 @@ export class HttpRequestService {
): Promise<Response> {
const timeout = args.timeout ?? 5000;
+ const parsedUrl = new URL(url);
+ const allowHttp = args.allowHttp || await isPrivateUrl(parsedUrl, this.lookup);
+ this.utilityService.assertUrl(parsedUrl, allowHttp);
+
const controller = new AbortController();
setTimeout(() => {
controller.abort();
@@ -318,7 +349,7 @@ export class HttpRequestService {
const isLocalAddressAllowed = args.isLocalAddressAllowed ?? false;
- const res = await fetch(url, {
+ const res = await fetch(parsedUrl, {
method: args.method ?? 'GET',
headers: {
'User-Agent': this.config.userAgent,
diff --git a/packages/backend/src/core/UtilityService.ts b/packages/backend/src/core/UtilityService.ts
index 3098367392..a90774cf59 100644
--- a/packages/backend/src/core/UtilityService.ts
+++ b/packages/backend/src/core/UtilityService.ts
@@ -10,7 +10,10 @@ import psl from 'psl';
import { DI } from '@/di-symbols.js';
import type { Config } from '@/config.js';
import { bindThis } from '@/decorators.js';
-import { MiMeta } from '@/models/Meta.js';
+import type { MiMeta } from '@/models/Meta.js';
+import type { MiInstance } from '@/models/Instance.js';
+import { IdentifiableError } from '@/misc/identifiable-error.js';
+import { EnvService } from '@/core/EnvService.js';
@Injectable()
export class UtilityService {
@@ -20,6 +23,8 @@ export class UtilityService {
@Inject(DI.meta)
private meta: MiMeta,
+
+ private readonly envService: EnvService,
) {
}
@@ -181,8 +186,8 @@ export class UtilityService {
}
@bindThis
- public punyHostPSLDomain(url: string): string {
- const urlObj = new URL(url);
+ public punyHostPSLDomain(url: string | URL): string {
+ const urlObj = typeof(url) === 'object' ? url : new URL(url);
const hostname = urlObj.hostname;
const domain = this.specialSuffix(hostname) ?? psl.get(hostname) ?? hostname;
const host = `${this.toPuny(domain)}${urlObj.port.length > 0 ? ':' + urlObj.port : ''}`;
@@ -213,4 +218,52 @@ export class UtilityService {
return '';
}
}
+
+ /**
+ * Verifies that a provided URL is in a format acceptable for federation.
+ * @throws {IdentifiableError} If URL cannot be parsed
+ * @throws {IdentifiableError} If URL is not HTTPS
+ * @throws {IdentifiableError} If URL contains credentials
+ */
+ @bindThis
+ public assertUrl(url: string | URL, allowHttp?: boolean): URL | never {
+ // If string, parse and validate
+ if (typeof(url) === 'string') {
+ try {
+ url = new URL(url);
+ } catch {
+ throw new IdentifiableError('0bedd29b-e3bf-4604-af51-d3352e2518af', `invalid url ${url}: not a valid URL`);
+ }
+ }
+
+ // Must be HTTPS
+ if (!this.checkHttps(url, allowHttp)) {
+ throw new IdentifiableError('0bedd29b-e3bf-4604-af51-d3352e2518af', `invalid url ${url}: unsupported protocol ${url.protocol}`);
+ }
+
+ // Must not have credentials
+ if (url.username || url.password) {
+ throw new IdentifiableError('0bedd29b-e3bf-4604-af51-d3352e2518af', `invalid url ${url}: contains embedded credentials`);
+ }
+
+ return url;
+ }
+
+ /**
+ * Checks if the URL contains HTTPS.
+ * Additionally, allows HTTP in non-production environments.
+ * Based on check-https.ts.
+ */
+ @bindThis
+ public checkHttps(url: string | URL, allowHttp = false): boolean {
+ const isNonProd = this.envService.env.NODE_ENV !== 'production';
+
+ try {
+ const proto = new URL(url).protocol;
+ return proto === 'https:' || (proto === 'http:' && (isNonProd || allowHttp));
+ } catch {
+ // Invalid URLs don't "count" as HTTPS
+ return false;
+ }
+ }
}
diff --git a/packages/backend/src/core/activitypub/ApRequestService.ts b/packages/backend/src/core/activitypub/ApRequestService.ts
index e4db9b237c..7669ce9669 100644
--- a/packages/backend/src/core/activitypub/ApRequestService.ts
+++ b/packages/backend/src/core/activitypub/ApRequestService.ts
@@ -157,8 +157,6 @@ export class ApRequestService {
@bindThis
public async signedPost(user: { id: MiUser['id'] }, url: string, object: unknown, digest?: string): Promise<void> {
- this.apUtilityService.assertApUrl(url);
-
const body = typeof object === 'string' ? object : JSON.stringify(object);
const keypair = await this.userKeypairService.getUserKeypair(user.id);
@@ -191,8 +189,6 @@ export class ApRequestService {
*/
@bindThis
public async signedGet(url: string, user: { id: MiUser['id'] }, allowAnonymous = false, followAlternate?: boolean): Promise<IObjectWithId> {
- this.apUtilityService.assertApUrl(url);
-
const _followAlternate = followAlternate ?? true;
const keypair = await this.userKeypairService.getUserKeypair(user.id);
diff --git a/packages/backend/src/core/activitypub/ApUtilityService.ts b/packages/backend/src/core/activitypub/ApUtilityService.ts
index 227dc3b9b3..26ea0cd632 100644
--- a/packages/backend/src/core/activitypub/ApUtilityService.ts
+++ b/packages/backend/src/core/activitypub/ApUtilityService.ts
@@ -7,20 +7,29 @@ import { Injectable } from '@nestjs/common';
import { UtilityService } from '@/core/UtilityService.js';
import { IdentifiableError } from '@/misc/identifiable-error.js';
import { toArray } from '@/misc/prelude/array.js';
-import { EnvService } from '@/core/EnvService.js';
-import { getApId, getOneApHrefNullable, IObject } from './type.js';
+import { getApId, getNullableApId, getOneApHrefNullable } from '@/core/activitypub/type.js';
+import type { IObject, IObjectWithId } from '@/core/activitypub/type.js';
+import { bindThis } from '@/decorators.js';
+import { renderInlineError } from '@/misc/render-inline-error.js';
+import { LoggerService } from '@/core/LoggerService.js';
+import type Logger from '@/logger.js';
@Injectable()
export class ApUtilityService {
+ private readonly logger: Logger;
+
constructor(
private readonly utilityService: UtilityService,
- private readonly envService: EnvService,
- ) {}
+ loggerService: LoggerService,
+ ) {
+ this.logger = loggerService.getLogger('ap-utility');
+ }
/**
* Verifies that the object's ID has the same authority as the provided URL.
* Returns on success, throws on any validation error.
*/
+ @bindThis
public assertIdMatchesUrlAuthority(object: IObject, url: string): void {
// This throws if the ID is missing or invalid, but that's ok.
// Anonymous objects are impossible to verify, so we don't allow fetching them.
@@ -36,11 +45,15 @@ export class ApUtilityService {
/**
* Checks if two URLs have the same host authority
*/
+ @bindThis
public haveSameAuthority(url1: string, url2: string): boolean {
if (url1 === url2) return true;
- const authority1 = this.utilityService.punyHostPSLDomain(url1);
- const authority2 = this.utilityService.punyHostPSLDomain(url2);
+ const parsed1 = this.utilityService.assertUrl(url1);
+ const parsed2 = this.utilityService.assertUrl(url2);
+
+ const authority1 = this.utilityService.punyHostPSLDomain(parsed1);
+ const authority2 = this.utilityService.punyHostPSLDomain(parsed2);
return authority1 === authority2;
}
@@ -50,6 +63,7 @@ export class ApUtilityService {
* @throws {IdentifiableError} if object does not have an ID
* @returns the best URL, or null if none were found
*/
+ @bindThis
public findBestObjectUrl(object: IObject): string | null {
const targetUrl = getApId(object);
const targetAuthority = this.utilityService.punyHostPSLDomain(targetUrl);
@@ -63,12 +77,16 @@ export class ApUtilityService {
: undefined,
}))
.filter(({ url, type }) => {
- if (!url) return false;
- if (!this.checkHttps(url)) return false;
- if (!isAcceptableUrlType(type)) return false;
+ try {
+ if (!url) return false;
+ if (!isAcceptableUrlType(type)) return false;
+ const parsed = this.utilityService.assertUrl(url);
- const urlAuthority = this.utilityService.punyHostPSLDomain(url);
- return urlAuthority === targetAuthority;
+ const urlAuthority = this.utilityService.punyHostPSLDomain(parsed);
+ return urlAuthority === targetAuthority;
+ } catch {
+ return false;
+ }
})
.sort((a, b) => {
return rankUrlType(a.type) - rankUrlType(b.type);
@@ -78,41 +96,72 @@ export class ApUtilityService {
}
/**
- * Verifies that a provided URL is in a format acceptable for federation.
- * @throws {IdentifiableError} If URL cannot be parsed
- * @throws {IdentifiableError} If URL is not HTTPS
+ * Sanitizes an inline / nested Object property within an AP object.
+ *
+ * Returns true if the property contains a valid string URL, object w/ valid ID, or an array containing one of those.
+ * Returns false and erases the property if it doesn't contain a valid value.
+ *
+ * Arrays are automatically flattened.
+ * Falsy values (including null) are collapsed to undefined.
+ * @param obj Object containing the property to validate
+ * @param key Key of the property in obj
+ * @param parentUri URI of the object that contains this inline object.
+ * @param parentHost PSL host of parentUri
+ * @param keyPath If obj is *itself* a nested object, set this to the property path from root to obj (including the trailing '.'). This does not affect the logic, but improves the clarity of logs.
*/
- public assertApUrl(url: string | URL): void {
- // If string, parse and validate
- if (typeof(url) === 'string') {
- try {
- url = new URL(url);
- } catch {
- throw new IdentifiableError('0bedd29b-e3bf-4604-af51-d3352e2518af', `invalid AP url ${url}: not a valid URL`);
- }
+ @bindThis
+ public sanitizeInlineObject<Key extends string>(obj: Partial<Record<Key, string | { id?: string } | (string | { id?: string })[]>>, key: Key, parentUri: string | URL, parentHost: string, keyPath = ''): obj is Partial<Record<Key, string | { id: string }>> {
+ let value: unknown = obj[key];
+
+ // Unpack arrays
+ if (Array.isArray(value)) {
+ value = value[0];
}
- // Must be HTTPS
- if (!this.checkHttps(url)) {
- throw new IdentifiableError('0bedd29b-e3bf-4604-af51-d3352e2518af', `invalid AP url ${url}: unsupported protocol ${url.protocol}`);
+ // Clear the value - we'll add it back once we have a confirmed ID
+ obj[key] = undefined;
+
+ // Collapse falsy values to undefined
+ if (!value) {
+ return false;
}
- }
- /**
- * Checks if the URL contains HTTPS.
- * Additionally, allows HTTP in non-production environments.
- * Based on check-https.ts.
- */
- private checkHttps(url: string | URL): boolean {
- const isNonProd = this.envService.env.NODE_ENV !== 'production';
+ // Exclude nested arrays
+ if (Array.isArray(value)) {
+ this.logger.warn(`Excluding ${keyPath}${key} from object ${parentUri}: nested arrays are prohibited`);
+ return false;
+ }
+
+ // Exclude incorrect types
+ if (typeof(value) !== 'string' && typeof(value) !== 'object') {
+ this.logger.warn(`Excluding ${keyPath}${key} from object ${parentUri}: incorrect type ${typeof(value)}`);
+ return false;
+ }
+
+ const valueId = getNullableApId(value);
+ if (!valueId) {
+ // Exclude missing ID
+ this.logger.warn(`Excluding ${keyPath}${key} from object ${parentUri}: missing or invalid ID`);
+ return false;
+ }
try {
- const proto = new URL(url).protocol;
- return proto === 'https:' || (proto === 'http:' && isNonProd);
- } catch {
- // Invalid URLs don't "count" as HTTPS
+ const parsed = this.utilityService.assertUrl(valueId);
+ const parsedHost = this.utilityService.punyHostPSLDomain(parsed);
+ if (parsedHost !== parentHost) {
+ // Exclude wrong host
+ this.logger.warn(`Excluding ${keyPath}${key} from object ${parentUri}: wrong host in ${valueId} (got ${parsedHost}, expected ${parentHost})`);
+ return false;
+ }
+ } catch (err) {
+ // Exclude invalid URLs
+ this.logger.warn(`Excluding ${keyPath}${key} from object ${parentUri}: invalid URL ${valueId}: ${renderInlineError(err)}`);
return false;
}
+
+ // Success - store the sanitized value and return
+ obj[key] = value as string | IObjectWithId;
+ return true;
}
}
diff --git a/packages/backend/src/core/activitypub/models/ApNoteService.ts b/packages/backend/src/core/activitypub/models/ApNoteService.ts
index 2a28405121..a67b02a9dc 100644
--- a/packages/backend/src/core/activitypub/models/ApNoteService.ts
+++ b/packages/backend/src/core/activitypub/models/ApNoteService.ts
@@ -96,7 +96,7 @@ export class ApNoteService {
actor?: MiRemoteUser,
user?: MiRemoteUser,
): Error | null {
- this.apUtilityService.assertApUrl(uri);
+ this.utilityService.assertUrl(uri);
const expectHost = this.utilityService.extractDbHost(uri);
const apType = getApType(object);
diff --git a/packages/backend/src/core/activitypub/models/ApPersonService.ts b/packages/backend/src/core/activitypub/models/ApPersonService.ts
index bc602bbd5b..f1a2522c04 100644
--- a/packages/backend/src/core/activitypub/models/ApPersonService.ts
+++ b/packages/backend/src/core/activitypub/models/ApPersonService.ts
@@ -45,6 +45,7 @@ import { HttpRequestService } from '@/core/HttpRequestService.js';
import { verifyFieldLinks } from '@/misc/verify-field-link.js';
import { isRetryableError } from '@/misc/is-retryable-error.js';
import { renderInlineError } from '@/misc/render-inline-error.js';
+import { IdentifiableError } from '@/misc/identifiable-error.js';
import { getApId, getApType, isActor, isCollection, isCollectionOrOrderedCollection, isPropertyValue } from '../type.js';
import { extractApHashtags } from './tag.js';
import type { OnModuleInit } from '@nestjs/common';
@@ -55,7 +56,6 @@ import type { ApLoggerService } from '../ApLoggerService.js';
import type { ApImageService } from './ApImageService.js';
import type { IActor, ICollection, IObject, IOrderedCollection } from '../type.js';
-import { IdentifiableError } from '@/misc/identifiable-error.js';
const nameLength = 128;
const summaryLength = 2048;
@@ -155,89 +155,88 @@ export class ApPersonService implements OnModuleInit, OnApplicationShutdown {
*/
@bindThis
private validateActor(x: IObject, uri: string): IActor {
- this.apUtilityService.assertApUrl(uri);
- const expectHost = this.utilityService.punyHostPSLDomain(uri);
+ const parsedUri = this.utilityService.assertUrl(uri);
+ const expectHost = this.utilityService.punyHostPSLDomain(parsedUri);
+ // Validate type
if (!isActor(x)) {
throw new UnrecoverableError(`invalid Actor ${uri}: unknown type '${x.type}'`);
}
- if (!(typeof x.id === 'string' && x.id.length > 0)) {
- throw new UnrecoverableError(`invalid Actor ${uri}: wrong id type`);
+ // Validate id
+ if (!x.id) {
+ throw new UnrecoverableError(`invalid Actor ${uri}: missing id`);
+ }
+ if (typeof(x.id) !== 'string') {
+ throw new UnrecoverableError(`invalid Actor ${uri}: wrong id type ${typeof(x.id)}`);
+ }
+ const parsedId = this.utilityService.assertUrl(x.id);
+ const idHost = this.utilityService.punyHostPSLDomain(parsedId);
+ if (idHost !== expectHost) {
+ throw new UnrecoverableError(`invalid Actor ${uri}: wrong host in id ${x.id} (got ${parsedId}, expected ${expectHost})`);
}
- if (!(typeof x.inbox === 'string' && x.inbox.length > 0)) {
- throw new UnrecoverableError(`invalid Actor ${uri}: wrong inbox type`);
+ // Validate inbox
+ this.apUtilityService.sanitizeInlineObject(x, 'inbox', parsedUri, expectHost);
+ if (!x.inbox || typeof(x.inbox) !== 'string') {
+ throw new UnrecoverableError(`invalid Actor ${uri}: missing or invalid inbox ${x.inbox}`);
}
- this.apUtilityService.assertApUrl(x.inbox);
- const inboxHost = this.utilityService.punyHostPSLDomain(x.inbox);
- if (inboxHost !== expectHost) {
- throw new UnrecoverableError(`invalid Actor ${uri}: wrong inbox host ${inboxHost}`);
+ // Sanitize sharedInbox
+ this.apUtilityService.sanitizeInlineObject(x, 'sharedInbox', parsedUri, expectHost);
+
+ // Sanitize endpoints object
+ if (typeof(x.endpoints) === 'object') {
+ x.endpoints = {
+ sharedInbox: x.endpoints.sharedInbox,
+ };
+ } else {
+ x.endpoints = undefined;
}
- const sharedInboxObject = x.sharedInbox ?? (x.endpoints ? x.endpoints.sharedInbox : undefined);
- if (sharedInboxObject != null) {
- const sharedInbox = getApId(sharedInboxObject);
- this.apUtilityService.assertApUrl(sharedInbox);
- if (!(typeof sharedInbox === 'string' && sharedInbox.length > 0 && this.utilityService.punyHostPSLDomain(sharedInbox) === expectHost)) {
- throw new UnrecoverableError(`invalid Actor ${uri}: wrong shared inbox ${sharedInbox}`);
+ // Sanitize endpoints.sharedInbox
+ if (x.endpoints) {
+ this.apUtilityService.sanitizeInlineObject(x.endpoints, 'sharedInbox', parsedUri, expectHost, 'endpoints.');
+
+ if (!x.endpoints.sharedInbox) {
+ x.endpoints = undefined;
}
}
- for (const collection of ['outbox', 'followers', 'following'] as (keyof IActor)[]) {
- const xCollection = (x as IActor)[collection];
- if (xCollection != null) {
- const collectionUri = getApId(xCollection);
- if (typeof collectionUri === 'string' && collectionUri.length > 0) {
- this.apUtilityService.assertApUrl(collectionUri);
- if (this.utilityService.punyHostPSLDomain(collectionUri) !== expectHost) {
- throw new UnrecoverableError(`invalid Actor ${uri}: wrong ${collection} host ${collectionUri}`);
- }
- } else if (collectionUri != null) {
- throw new UnrecoverableError(`invalid Actor ${uri}: wrong ${collection} type`);
- }
- }
+ // Sanitize collections
+ for (const collection of ['outbox', 'followers', 'following', 'featured'] as const) {
+ this.apUtilityService.sanitizeInlineObject(x, collection, parsedUri, expectHost);
}
+ // Validate username
if (!(typeof x.preferredUsername === 'string' && x.preferredUsername.length > 0 && x.preferredUsername.length <= 128 && /^\w([\w-.]*\w)?$/.test(x.preferredUsername))) {
throw new UnrecoverableError(`invalid Actor ${uri}: wrong username`);
}
+ // Sanitize name
// These fields are only informational, and some AP software allows these
// fields to be very long. If they are too long, we cut them off. This way
// we can at least see these users and their activities.
- if (x.name) {
- if (!(typeof x.name === 'string' && x.name.length > 0)) {
- throw new UnrecoverableError(`invalid Actor ${uri}: wrong name`);
- }
- x.name = truncate(x.name, nameLength);
- } else if (x.name === '') {
- // Mastodon emits empty string when the name is not set.
+ if (!x.name) {
x.name = undefined;
- }
- if (x.summary) {
- if (!(typeof x.summary === 'string' && x.summary.length > 0)) {
- throw new UnrecoverableError(`invalid Actor ${uri}: wrong summary`);
- }
- x.summary = truncate(x.summary, summaryLength);
+ } else if (typeof(x.name) !== 'string') {
+ this.logger.warn(`Excluding name from object ${uri}: incorrect type ${typeof(x)}`);
+ x.name = undefined;
+ } else {
+ x.name = truncate(x.name, nameLength);
}
- const idHost = this.utilityService.punyHostPSLDomain(x.id);
- if (idHost !== expectHost) {
- throw new UnrecoverableError(`invalid Actor ${uri}: wrong id ${x.id}`);
+ // Sanitize summary
+ if (!x.summary) {
+ x.summary = undefined;
+ } else if (typeof(x.summary) !== 'string') {
+ this.logger.warn(`Excluding summary from object ${uri}: incorrect type ${typeof(x)}`);
+ } else {
+ x.summary = truncate(x.summary, summaryLength);
}
- if (x.publicKey) {
- if (typeof x.publicKey.id !== 'string') {
- throw new UnrecoverableError(`invalid Actor ${uri}: wrong publicKey.id type`);
- }
-
- const publicKeyIdHost = this.utilityService.punyHostPSLDomain(x.publicKey.id);
- if (publicKeyIdHost !== expectHost) {
- throw new UnrecoverableError(`invalid Actor ${uri}: wrong publicKey.id ${x.publicKey.id}`);
- }
- }
+ // Sanitize publicKey
+ this.apUtilityService.sanitizeInlineObject(x, 'publicKey', parsedUri, expectHost);
return x;
}
diff --git a/packages/backend/src/core/activitypub/type.ts b/packages/backend/src/core/activitypub/type.ts
index 554420d670..ebe29a4fe6 100644
--- a/packages/backend/src/core/activitypub/type.ts
+++ b/packages/backend/src/core/activitypub/type.ts
@@ -86,7 +86,7 @@ export function getOneApId(value: ApObject): string {
/**
* Get ActivityStreams Object id
*/
-export function getApId(value: string | IObject | [string | IObject], sourceForLogs?: string): string {
+export function getApId(value: unknown | [unknown] | unknown[], sourceForLogs?: string): string {
const id = getNullableApId(value);
if (id == null) {
@@ -102,7 +102,7 @@ export function getApId(value: string | IObject | [string | IObject], sourceForL
/**
* Get ActivityStreams Object id, or null if not present
*/
-export function getNullableApId(source: string | IObject | [string | IObject]): string | null {
+export function getNullableApId(source: unknown | [unknown] | unknown[]): string | null {
const value: unknown = fromTuple(source);
if (value != null) {
@@ -276,7 +276,7 @@ export interface IActor extends IObject {
followers?: string | ICollection | IOrderedCollection;
following?: string | ICollection | IOrderedCollection;
featured?: string | IOrderedCollection;
- outbox: string | IOrderedCollection;
+ outbox?: string | IOrderedCollection;
endpoints?: {
sharedInbox?: string;
};
diff --git a/packages/backend/src/misc/captcha-error.ts b/packages/backend/src/misc/captcha-error.ts
new file mode 100644
index 0000000000..217018ec68
--- /dev/null
+++ b/packages/backend/src/misc/captcha-error.ts
@@ -0,0 +1,18 @@
+/*
+ * SPDX-FileCopyrightText: syuilo and misskey-project
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+
+import type { CaptchaErrorCode } from '@/core/CaptchaService.js';
+
+export class CaptchaError extends Error {
+ public readonly code: CaptchaErrorCode;
+ public readonly cause?: unknown;
+
+ constructor(code: CaptchaErrorCode, message: string, cause?: unknown) {
+ super(message, cause ? { cause } : undefined);
+ this.code = code;
+ this.cause = cause;
+ this.name = 'CaptchaError';
+ }
+}
diff --git a/packages/backend/src/misc/render-inline-error.ts b/packages/backend/src/misc/render-inline-error.ts
index 07f9f3068e..886efcb86e 100644
--- a/packages/backend/src/misc/render-inline-error.ts
+++ b/packages/backend/src/misc/render-inline-error.ts
@@ -5,7 +5,7 @@
import { IdentifiableError } from '@/misc/identifiable-error.js';
import { StatusError } from '@/misc/status-error.js';
-import { CaptchaError } from '@/core/CaptchaService.js';
+import { CaptchaError } from '@/misc/captcha-error.js';
export function renderInlineError(err: unknown): string {
const parts: string[] = [];
diff --git a/packages/backend/src/misc/verify-field-link.ts b/packages/backend/src/misc/verify-field-link.ts
index 6a3c950059..31a356be37 100644
--- a/packages/backend/src/misc/verify-field-link.ts
+++ b/packages/backend/src/misc/verify-field-link.ts
@@ -10,8 +10,9 @@ type Field = { name: string, value: string };
export async function verifyFieldLinks(fields: Field[], profileUrls: string[], httpRequestService: HttpRequestService): Promise<string[]> {
const verified_links = [];
- for (const field_url of fields.filter(x => URL.canParse(x.value) && ['http:', 'https:'].includes((new URL(x.value).protocol)))) {
+ for (const field_url of fields) {
try {
+ // getHtml validates the input URL, so we can safely pass in untrusted values
const html = await httpRequestService.getHtml(field_url.value);
const doc = cheerio(html);
diff --git a/packages/backend/test/unit/FetchInstanceMetadataService.ts b/packages/backend/test/unit/FetchInstanceMetadataService.ts
index 1e3605aafc..812ee38703 100644
--- a/packages/backend/test/unit/FetchInstanceMetadataService.ts
+++ b/packages/backend/test/unit/FetchInstanceMetadataService.ts
@@ -16,6 +16,7 @@ import { HttpRequestService } from '@/core/HttpRequestService.js';
import { LoggerService } from '@/core/LoggerService.js';
import { UtilityService } from '@/core/UtilityService.js';
import { IdService } from '@/core/IdService.js';
+import { EnvService } from '@/core/EnvService.js';
import { DI } from '@/di-symbols.js';
function mockRedis() {
@@ -46,6 +47,7 @@ describe('FetchInstanceMetadataService', () => {
LoggerService,
UtilityService,
IdService,
+ EnvService,
],
})
.useMocker((token) => {
diff --git a/packages/backend/test/unit/core/HttpRequestService.ts b/packages/backend/test/unit/core/HttpRequestService.ts
index a2f4604e7b..ccce32ffee 100644
--- a/packages/backend/test/unit/core/HttpRequestService.ts
+++ b/packages/backend/test/unit/core/HttpRequestService.ts
@@ -3,11 +3,11 @@
* SPDX-License-Identifier: AGPL-3.0-only
*/
-import { jest } from '@jest/globals';
+import { describe, jest } from '@jest/globals';
import type { Mock } from 'jest-mock';
import type { PrivateNetwork } from '@/config.js';
import type { Socket } from 'net';
-import { HttpRequestService, isPrivateIp, validateSocketConnect } from '@/core/HttpRequestService.js';
+import { HttpRequestService, isAllowedPrivateIp, isPrivateUrl, resolveIp, validateSocketConnect } from '@/core/HttpRequestService.js';
import { parsePrivateNetworks } from '@/config.js';
describe(HttpRequestService, () => {
@@ -21,38 +21,85 @@ describe(HttpRequestService, () => {
]);
});
- describe('isPrivateIp', () => {
+ describe(isAllowedPrivateIp, () => {
it('should return false when ip public', () => {
- const result = isPrivateIp(allowedPrivateNetworks, '74.125.127.100', 80);
+ const result = isAllowedPrivateIp(allowedPrivateNetworks, '74.125.127.100', 80);
expect(result).toBeFalsy();
});
it('should return false when ip private and port matches', () => {
- const result = isPrivateIp(allowedPrivateNetworks, '127.0.0.1', 1);
+ const result = isAllowedPrivateIp(allowedPrivateNetworks, '127.0.0.1', 1);
expect(result).toBeFalsy();
});
it('should return false when ip private and all ports undefined', () => {
- const result = isPrivateIp(allowedPrivateNetworks, '10.0.0.1', undefined);
+ const result = isAllowedPrivateIp(allowedPrivateNetworks, '10.0.0.1', undefined);
expect(result).toBeFalsy();
});
it('should return true when ip private and no ports specified', () => {
- const result = isPrivateIp(allowedPrivateNetworks, '10.0.0.2', 80);
+ const result = isAllowedPrivateIp(allowedPrivateNetworks, '10.0.0.2', 80);
expect(result).toBeTruthy();
});
it('should return true when ip private and port does not match', () => {
- const result = isPrivateIp(allowedPrivateNetworks, '127.0.0.1', 80);
+ const result = isAllowedPrivateIp(allowedPrivateNetworks, '127.0.0.1', 80);
expect(result).toBeTruthy();
});
it('should return true when ip private and port is null but ports are specified', () => {
- const result = isPrivateIp(allowedPrivateNetworks, '127.0.0.1', undefined);
+ const result = isAllowedPrivateIp(allowedPrivateNetworks, '127.0.0.1', undefined);
expect(result).toBeTruthy();
});
});
+ const fakeLookup = (host: string, _: unknown, callback: (err: Error | null, ip: string) => void) => {
+ if (host === 'localhost') {
+ callback(null, '127.0.0.1');
+ } else {
+ callback(null, '23.192.228.80');
+ }
+ };
+
+ describe(resolveIp, () => {
+ it('should parse inline IPs', async () => {
+ const result = await resolveIp(new URL('https://10.0.0.1'), fakeLookup);
+ expect(result.toString()).toEqual('10.0.0.1');
+ });
+
+ it('should resolve domain names', async () => {
+ const result = await resolveIp(new URL('https://localhost'), fakeLookup);
+ expect(result.toString()).toEqual('127.0.0.1');
+ });
+ });
+
+ describe(isPrivateUrl, () => {
+ it('should return false when URL is public host', async () => {
+ const result = await isPrivateUrl(new URL('https://example.com'), fakeLookup);
+ expect(result).toBe(false);
+ });
+
+ it('should return true when URL is private host', async () => {
+ const result = await isPrivateUrl(new URL('https://localhost'), fakeLookup);
+ expect(result).toBe(true);
+ });
+
+ it('should return false when IP is public', async () => {
+ const result = await isPrivateUrl(new URL('https://23.192.228.80'), fakeLookup);
+ expect(result).toBe(false);
+ });
+
+ it('should return true when IP is private', async () => {
+ const result = await isPrivateUrl(new URL('https://127.0.0.1'), fakeLookup);
+ expect(result).toBe(true);
+ });
+
+ it('should return true when IP is private with port and path', async () => {
+ const result = await isPrivateUrl(new URL('https://127.0.0.1:443/some/path'), fakeLookup);
+ expect(result).toBe(true);
+ });
+ });
+
describe('validateSocketConnect', () => {
let fakeSocket: Socket;
let fakeSocketMutable: {
diff --git a/packages/backend/test/unit/core/activitypub/ApUtilityService.ts b/packages/backend/test/unit/core/activitypub/ApUtilityService.ts
index 325a94dc5a..7b564b1fdd 100644
--- a/packages/backend/test/unit/core/activitypub/ApUtilityService.ts
+++ b/packages/backend/test/unit/core/activitypub/ApUtilityService.ts
@@ -3,30 +3,52 @@
* SPDX-License-Identifier: AGPL-3.0-only
*/
-import type { UtilityService } from '@/core/UtilityService.js';
import type { IObject } from '@/core/activitypub/type.js';
import type { EnvService } from '@/core/EnvService.js';
+import type { MiMeta } from '@/models/Meta.js';
+import type { Config } from '@/config.js';
+import type { LoggerService } from '@/core/LoggerService.js';
+import Logger from '@/logger.js';
import { ApUtilityService } from '@/core/activitypub/ApUtilityService.js';
+import { UtilityService } from '@/core/UtilityService.js';
describe(ApUtilityService, () => {
let serviceUnderTest: ApUtilityService;
let env: Record<string, string>;
beforeEach(() => {
- const utilityService = {
- punyHostPSLDomain(input: string) {
- const host = new URL(input).host;
- const parts = host.split('.');
- return `${parts[parts.length - 2]}.${parts[parts.length - 1]}`;
- },
- } as unknown as UtilityService;
-
env = {};
const envService = {
env,
} as unknown as EnvService;
- serviceUnderTest = new ApUtilityService(utilityService, envService);
+ const config = {
+ host: 'example.com',
+ blockedHosts: [],
+ silencedHosts: [],
+ mediaSilencedHosts: [],
+ federationHosts: [],
+ bubbleInstances: [],
+ deliverSuspendedSoftware: [],
+ federation: 'all',
+ } as unknown as Config;
+ const meta = {
+
+ } as MiMeta;
+
+ const utilityService = new UtilityService(config, meta, envService);
+
+ const loggerService = {
+ getLogger(domain: string) {
+ const logger = new Logger(domain);
+ Object.defineProperty(logger, 'log', {
+ value: () => {},
+ });
+ return logger;
+ },
+ } as unknown as LoggerService;
+
+ serviceUnderTest = new ApUtilityService(utilityService, loggerService);
});
describe('assertIdMatchesUrlAuthority', () => {
@@ -351,4 +373,102 @@ describe(ApUtilityService, () => {
expect(result).toBe('http://example.com/1');
});
});
+
+ describe('sanitizeInlineObject', () => {
+ it('should exclude nested arrays', () => {
+ const input = {
+ test: [[]] as unknown as string[],
+ };
+
+ const result = serviceUnderTest.sanitizeInlineObject(input, 'test', 'https://example.com/actor', 'example.com');
+
+ expect(result).toBe(false);
+ });
+
+ it('should exclude incorrect type', () => {
+ const input = {
+ test: 0 as unknown as string,
+ };
+
+ const result = serviceUnderTest.sanitizeInlineObject(input, 'test', 'https://example.com/actor', 'example.com');
+
+ expect(result).toBe(false);
+ });
+
+ it('should exclude missing ID', () => {
+ const input = {
+ test: {
+ id: undefined,
+ },
+ };
+
+ const result = serviceUnderTest.sanitizeInlineObject(input, 'test', 'https://example.com/actor', 'example.com');
+
+ expect(result).toBe(false);
+ });
+
+ it('should exclude wrong host', () => {
+ const input = {
+ test: 'https://wrong.com/object',
+ };
+
+ const result = serviceUnderTest.sanitizeInlineObject(input, 'test', 'https://example.com/actor', 'example.com');
+
+ expect(result).toBe(false);
+ });
+
+ it('should exclude invalid URLs', () => {
+ const input = {
+ test: 'https://user@example.com/object',
+ };
+
+ const result = serviceUnderTest.sanitizeInlineObject(input, 'test', 'https://example.com/actor', 'example.com');
+
+ expect(result).toBe(false);
+ });
+
+ it('should accept string', () => {
+ const input = {
+ test: 'https://example.com/object',
+ };
+
+ const result = serviceUnderTest.sanitizeInlineObject(input, 'test', 'https://example.com/actor', 'example.com');
+
+ expect(result).toBe(true);
+ });
+
+ it('should accept array of string', () => {
+ const input = {
+ test: ['https://example.com/object'],
+ };
+
+ const result = serviceUnderTest.sanitizeInlineObject(input, 'test', 'https://example.com/actor', 'example.com');
+
+ expect(result).toBe(true);
+ });
+
+ it('should accept object', () => {
+ const input = {
+ test: {
+ id: 'https://example.com/object',
+ },
+ };
+
+ const result = serviceUnderTest.sanitizeInlineObject(input, 'test', 'https://example.com/actor', 'example.com');
+
+ expect(result).toBe(true);
+ });
+
+ it('should accept array of object', () => {
+ const input = {
+ test: [{
+ id: 'https://example.com/object',
+ }],
+ };
+
+ const result = serviceUnderTest.sanitizeInlineObject(input, 'test', 'https://example.com/actor', 'example.com');
+
+ expect(result).toBe(true);
+ });
+ });
});