merge: Improve URL validation (!1155)

View MR for information: https://activitypub.software/TransFem-org/Sharkey/-/merge_requests/1155

Approved-by: dakkar <dakkar@thenautilus.net>
Approved-by: Marie <github@yuugi.dev>

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) => {

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: {

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);
+		});
+	});
 });