From 982223ad38e428ca4e2269fff56bccd332ca0222 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Fri, 4 Jul 2025 12:16:18 -0400 Subject: validate all URLs before fetch --- packages/backend/src/core/HttpRequestService.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'packages/backend/src/core/HttpRequestService.ts') diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts index 151097095d..046b0dc244 100644 --- a/packages/backend/src/core/HttpRequestService.ts +++ b/packages/backend/src/core/HttpRequestService.ts @@ -17,7 +17,8 @@ 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'; @@ -132,6 +133,7 @@ export class HttpRequestService { @Inject(DI.config) private config: Config, private readonly apUtilityService: ApUtilityService, + private readonly utilityService: UtilityService, ) { const cache = new CacheableLookup({ maxTtl: 3600, // 1hours @@ -236,8 +238,6 @@ export class HttpRequestService { @bindThis public async getActivityJson(url: string, isLocalAddressAllowed = false, allowAnonymous = false): Promise { - this.apUtilityService.assertApUrl(url); - const res = await this.send(url, { method: 'GET', headers: { @@ -311,6 +311,8 @@ export class HttpRequestService { ): Promise { const timeout = args.timeout ?? 5000; + this.utilityService.assertUrl(url); + const controller = new AbortController(); setTimeout(() => { controller.abort(); -- cgit v1.2.3-freya From 3c59a7ae01f7ea7094178cf961c3f4a668672f08 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Tue, 8 Jul 2025 11:27:31 -0400 Subject: allow HTTP connections to private IPs --- packages/backend/src/core/HttpRequestService.ts | 17 +++++++++++--- packages/backend/src/core/UtilityService.ts | 8 +++---- .../backend/test/unit/core/HttpRequestService.ts | 26 ++++++++++++++++++++-- 3 files changed, 42 insertions(+), 9 deletions(-) (limited to 'packages/backend/src/core/HttpRequestService.ts') diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts index 046b0dc244..bbc127fa86 100644 --- a/packages/backend/src/core/HttpRequestService.ts +++ b/packages/backend/src/core/HttpRequestService.ts @@ -20,7 +20,6 @@ import type { IObject, IObjectWithId } from '@/core/activitypub/type.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 = { @@ -28,6 +27,15 @@ export type HttpRequestSendOptions = { validators?: ((res: Response) => void)[]; }; +export function isPrivateUrl(url: URL): boolean { + if (!ipaddr.isValid(url.hostname)) { + return false; + } + + const ip = ipaddr.parse(url.hostname); + return ip.range() !== 'unicast'; +} + export function isPrivateIp(allowedPrivateNetworks: PrivateNetwork[] | undefined, ip: string, port?: number): boolean { const parsedIp = ipaddr.parse(ip); @@ -303,6 +311,7 @@ export class HttpRequestService { timeout?: number, size?: number, isLocalAddressAllowed?: boolean, + allowHttp?: boolean, } = {}, extra: HttpRequestSendOptions = { throwErrorWhenResponseNotOk: true, @@ -311,7 +320,9 @@ export class HttpRequestService { ): Promise { const timeout = args.timeout ?? 5000; - this.utilityService.assertUrl(url); + const parsedUrl = new URL(url); + const allowHttp = args.allowHttp || isPrivateUrl(parsedUrl); + this.utilityService.assertUrl(parsedUrl, allowHttp); const controller = new AbortController(); setTimeout(() => { @@ -320,7 +331,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 27d3cb7776..a90774cf59 100644 --- a/packages/backend/src/core/UtilityService.ts +++ b/packages/backend/src/core/UtilityService.ts @@ -226,7 +226,7 @@ export class UtilityService { * @throws {IdentifiableError} If URL contains credentials */ @bindThis - public assertUrl(url: string | URL): URL | never { + public assertUrl(url: string | URL, allowHttp?: boolean): URL | never { // If string, parse and validate if (typeof(url) === 'string') { try { @@ -237,7 +237,7 @@ export class UtilityService { } // Must be HTTPS - if (!this.checkHttps(url)) { + if (!this.checkHttps(url, allowHttp)) { throw new IdentifiableError('0bedd29b-e3bf-4604-af51-d3352e2518af', `invalid url ${url}: unsupported protocol ${url.protocol}`); } @@ -255,12 +255,12 @@ export class UtilityService { * Based on check-https.ts. */ @bindThis - public checkHttps(url: string | URL): boolean { + 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); + return proto === 'https:' || (proto === 'http:' && (isNonProd || allowHttp)); } catch { // Invalid URLs don't "count" as HTTPS return false; diff --git a/packages/backend/test/unit/core/HttpRequestService.ts b/packages/backend/test/unit/core/HttpRequestService.ts index a2f4604e7b..5aeeb1e26f 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, isPrivateIp, isPrivateUrl, validateSocketConnect } from '@/core/HttpRequestService.js'; import { parsePrivateNetworks } from '@/config.js'; describe(HttpRequestService, () => { @@ -53,6 +53,28 @@ describe(HttpRequestService, () => { }); }); + describe('isPrivateUrl', () => { + it('should return false when URL is not an IP', () => { + const result = isPrivateUrl(new URL('https://example.com')); + expect(result).toBe(false); + }); + + it('should return false when IP is public', () => { + const result = isPrivateUrl(new URL('https://23.192.228.80')); + expect(result).toBe(false); + }); + + it('should return true when IP is private', () => { + const result = isPrivateUrl(new URL('https://127.0.0.1')); + expect(result).toBe(true); + }); + + it('should return true when IP is private with port and path', () => { + const result = isPrivateUrl(new URL('https://127.0.0.1:443/some/path')); + expect(result).toBe(true); + }); + }); + describe('validateSocketConnect', () => { let fakeSocket: Socket; let fakeSocketMutable: { -- cgit v1.2.3-freya From 25622b536c982f717c72b0cf039322d45c36a55b Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Fri, 25 Jul 2025 16:28:53 -0400 Subject: resolve domain names when checking for private URLs --- packages/backend/src/core/HttpRequestService.ts | 30 ++++++++--- .../src/core/activitypub/models/ApPersonService.ts | 1 - .../backend/test/unit/core/HttpRequestService.ts | 60 ++++++++++++++++------ 3 files changed, 65 insertions(+), 26 deletions(-) (limited to 'packages/backend/src/core/HttpRequestService.ts') diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts index bbc127fa86..34aaada9f2 100644 --- a/packages/backend/src/core/HttpRequestService.ts +++ b/packages/backend/src/core/HttpRequestService.ts @@ -27,16 +27,27 @@ export type HttpRequestSendOptions = { validators?: ((res: Response) => void)[]; }; -export function isPrivateUrl(url: URL): boolean { - if (!ipaddr.isValid(url.hostname)) { - return false; +export async function isPrivateUrl(url: URL, lookup: net.LookupFunction): Promise { + 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 ip = ipaddr.parse(url.hostname); - return ip.range() !== 'unicast'; + const resolvedIp = await new Promise((resolve, reject) => { + lookup(url.hostname, {}, (err, address) => { + if (err) reject(err); + else resolve(address as string); + }); + }); + + return ipaddr.parse(resolvedIp); } -export function isPrivateIp(allowedPrivateNetworks: PrivateNetwork[] | undefined, ip: string, port?: number): boolean { +export function isAllowedPrivateIp(allowedPrivateNetworks: PrivateNetwork[] | undefined, ip: string, port?: number): boolean { const parsedIp = ipaddr.parse(ip); for (const { cidr, ports } of allowedPrivateNetworks ?? []) { @@ -53,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}`)); } } @@ -142,6 +153,7 @@ export class HttpRequestService { private config: Config, private readonly apUtilityService: ApUtilityService, private readonly utilityService: UtilityService, + private readonly lookup: net.LookupFunction, ) { const cache = new CacheableLookup({ maxTtl: 3600, // 1hours @@ -149,6 +161,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, @@ -321,7 +335,7 @@ export class HttpRequestService { const timeout = args.timeout ?? 5000; const parsedUrl = new URL(url); - const allowHttp = args.allowHttp || isPrivateUrl(parsedUrl); + const allowHttp = args.allowHttp || await isPrivateUrl(parsedUrl, this.lookup); this.utilityService.assertUrl(parsedUrl, allowHttp); const controller = new AbortController(); diff --git a/packages/backend/src/core/activitypub/models/ApPersonService.ts b/packages/backend/src/core/activitypub/models/ApPersonService.ts index b3fd17e7a0..30124949bb 100644 --- a/packages/backend/src/core/activitypub/models/ApPersonService.ts +++ b/packages/backend/src/core/activitypub/models/ApPersonService.ts @@ -56,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; diff --git a/packages/backend/test/unit/core/HttpRequestService.ts b/packages/backend/test/unit/core/HttpRequestService.ts index 5aeeb1e26f..0759306666 100644 --- a/packages/backend/test/unit/core/HttpRequestService.ts +++ b/packages/backend/test/unit/core/HttpRequestService.ts @@ -7,8 +7,9 @@ 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, isPrivateUrl, validateSocketConnect } from '@/core/HttpRequestService.js'; +import { HttpRequestService, isAllowedPrivateIp, isPrivateUrl, resolveIp, validateSocketConnect } from '@/core/HttpRequestService.js'; import { parsePrivateNetworks } from '@/config.js'; +import { IPv4 } from 'ipaddr.js'; describe(HttpRequestService, () => { let allowedPrivateNetworks: PrivateNetwork[] | undefined; @@ -21,56 +22,81 @@ 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(); }); }); - describe('isPrivateUrl', () => { - it('should return false when URL is not an IP', () => { - const result = isPrivateUrl(new URL('https://example.com')); + 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 false when IP is public', () => { - const result = isPrivateUrl(new URL('https://23.192.228.80')); + 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', () => { - const result = isPrivateUrl(new URL('https://127.0.0.1')); + 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', () => { - const result = isPrivateUrl(new URL('https://127.0.0.1:443/some/path')); + 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); }); }); -- cgit v1.2.3-freya From db15ac0fbb8bb4474f365a42a1df9056828d26b3 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Fri, 25 Jul 2025 16:32:54 -0400 Subject: fix DI error in HttpRequestService.ts --- packages/backend/src/core/HttpRequestService.ts | 6 +++++- packages/backend/test/unit/core/HttpRequestService.ts | 1 - 2 files changed, 5 insertions(+), 2 deletions(-) (limited to 'packages/backend/src/core/HttpRequestService.ts') diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts index 34aaada9f2..bd72fefe4f 100644 --- a/packages/backend/src/core/HttpRequestService.ts +++ b/packages/backend/src/core/HttpRequestService.ts @@ -148,12 +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, - private readonly lookup: net.LookupFunction, ) { const cache = new CacheableLookup({ maxTtl: 3600, // 1hours diff --git a/packages/backend/test/unit/core/HttpRequestService.ts b/packages/backend/test/unit/core/HttpRequestService.ts index 0759306666..ccce32ffee 100644 --- a/packages/backend/test/unit/core/HttpRequestService.ts +++ b/packages/backend/test/unit/core/HttpRequestService.ts @@ -9,7 +9,6 @@ import type { PrivateNetwork } from '@/config.js'; import type { Socket } from 'net'; import { HttpRequestService, isAllowedPrivateIp, isPrivateUrl, resolveIp, validateSocketConnect } from '@/core/HttpRequestService.js'; import { parsePrivateNetworks } from '@/config.js'; -import { IPv4 } from 'ipaddr.js'; describe(HttpRequestService, () => { let allowedPrivateNetworks: PrivateNetwork[] | undefined; -- cgit v1.2.3-freya