Merge branch 'refactor/is-safe-url' into 'develop'

refactor (backend): port isValidUrl to backend-rs


See merge request firefish/firefish!10795

.gitlab-ci.yml

@@ -16,8 +16,8 @@ workflow:
 cache:
   paths:
     - node_modules
-    - /cache/cargo/registry/index
+    # - /usr/local/cargo/registry/index
-    - /cache/cargo/registry/cache
+    # - /usr/local/cargo/registry/cache
     - target/debug/deps
     - target/debug/build
 
@@ -33,18 +33,18 @@ variables:
   CARGO_PROFILE_DEV_OPT_LEVEL: '0'
   CARGO_PROFILE_DEV_LTO: 'off'
   CARGO_PROFILE_DEV_DEBUG: 'none'
-  CARGO_HOME: '/cache/cargo'
 
 default:
   before_script:
-    - apt-get -y install curl
+    - mkdir -p "${CARGO_HOME}"
-    - curl -fsSL 'https://deb.nodesource.com/setup_18.x' | sudo -E bash -
+    - apt-get update && apt-get -y upgrade
-    - apt-get update && apt-get upgrade
+    - apt-get -y --no-install-recommends install curl
-    - apt-get install -y build-essential clang mold python3 perl nodejs postgresql-client
+    - curl -fsSL 'https://deb.nodesource.com/setup_18.x' | bash -
+    - apt-get install -y --no-install-recommends build-essential clang mold python3 perl nodejs postgresql-client
     - corepack enable
     - corepack prepare pnpm@latest --activate
     - cp .config/ci.yml .config/default.yml
-    - cp ci/cargo/config.toml "${CARGO_HOME}/config.toml"
+    - cp ci/cargo/config.toml /usr/local/cargo/config.toml
     - export PGPASSWORD="${POSTGRES_PASSWORD}"
     - psql --host postgres --user "${POSTGRES_USER}" --dbname "${POSTGRES_DB}" --command 'CREATE EXTENSION pgroonga'
 

packages/backend-rs/index.d.ts

@@ -268,6 +268,7 @@ export interface NoteLikeForGetNoteSummary {
   hasPoll: boolean
 }
 export function getNoteSummary(note: NoteLikeForGetNoteSummary): string
+export function isSafeUrl(url: string): boolean
 export function latestVersion(): Promise<string>
 export function toMastodonId(firefishId: string): string | null
 export function fromMastodonId(mastodonId: string): string | null

packages/backend-rs/index.js

@@ -310,7 +310,7 @@ if (!nativeBinding) {
   throw new Error(`Failed to load native binding`)
 }
 
-const { SECOND, MINUTE, HOUR, DAY, USER_ONLINE_THRESHOLD, USER_ACTIVE_THRESHOLD, FILE_TYPE_BROWSERSAFE, loadEnv, loadConfig, stringToAcct, acctToString, addNoteToAntenna, isBlockedServer, isSilencedServer, isAllowedServer, checkWordMute, getFullApAccount, isSelfHost, isSameOrigin, extractHost, toPuny, isUnicodeEmoji, sqlLikeEscape, safeForSql, formatMilliseconds, getImageSizeFromUrl, getNoteSummary, latestVersion, toMastodonId, fromMastodonId, fetchMeta, metaToPugArgs, nyaify, hashPassword, verifyPassword, isOldPasswordAlgorithm, decodeReaction, countReactions, toDbReaction, removeOldAttestationChallenges, AntennaSrcEnum, DriveFileUsageHintEnum, MutedNoteReasonEnum, NoteVisibilityEnum, NotificationTypeEnum, PageVisibilityEnum, PollNotevisibilityEnum, RelayStatusEnum, UserEmojimodpermEnum, UserProfileFfvisibilityEnum, UserProfileMutingnotificationtypesEnum, initializeRustLogger, fetchNodeinfo, nodeinfo_2_1, nodeinfo_2_0, Protocol, Inbound, Outbound, watchNote, unwatchNote, publishToChannelStream, ChatEvent, publishToChatStream, ChatIndexEvent, publishToChatIndexStream, publishToBroadcastStream, publishToGroupChatStream, publishToModerationStream, getTimestamp, genId, genIdAt, secureRndstr } = nativeBinding
+const { SECOND, MINUTE, HOUR, DAY, USER_ONLINE_THRESHOLD, USER_ACTIVE_THRESHOLD, FILE_TYPE_BROWSERSAFE, loadEnv, loadConfig, stringToAcct, acctToString, addNoteToAntenna, isBlockedServer, isSilencedServer, isAllowedServer, checkWordMute, getFullApAccount, isSelfHost, isSameOrigin, extractHost, toPuny, isUnicodeEmoji, sqlLikeEscape, safeForSql, formatMilliseconds, getImageSizeFromUrl, getNoteSummary, isSafeUrl, latestVersion, toMastodonId, fromMastodonId, fetchMeta, metaToPugArgs, nyaify, hashPassword, verifyPassword, isOldPasswordAlgorithm, decodeReaction, countReactions, toDbReaction, removeOldAttestationChallenges, AntennaSrcEnum, DriveFileUsageHintEnum, MutedNoteReasonEnum, NoteVisibilityEnum, NotificationTypeEnum, PageVisibilityEnum, PollNotevisibilityEnum, RelayStatusEnum, UserEmojimodpermEnum, UserProfileFfvisibilityEnum, UserProfileMutingnotificationtypesEnum, initializeRustLogger, fetchNodeinfo, nodeinfo_2_1, nodeinfo_2_0, Protocol, Inbound, Outbound, watchNote, unwatchNote, publishToChannelStream, ChatEvent, publishToChatStream, ChatIndexEvent, publishToChatIndexStream, publishToBroadcastStream, publishToGroupChatStream, publishToModerationStream, getTimestamp, genId, genIdAt, secureRndstr } = nativeBinding
 
 module.exports.SECOND = SECOND
 module.exports.MINUTE = MINUTE
@@ -339,6 +339,7 @@ module.exports.safeForSql = safeForSql
 module.exports.formatMilliseconds = formatMilliseconds
 module.exports.getImageSizeFromUrl = getImageSizeFromUrl
 module.exports.getNoteSummary = getNoteSummary
+module.exports.isSafeUrl = isSafeUrl
 module.exports.latestVersion = latestVersion
 module.exports.toMastodonId = toMastodonId
 module.exports.fromMastodonId = fromMastodonId

packages/backend-rs/src/misc/is_safe_url.rs

@@ -0,0 +1,34 @@
+#[crate::export]
+pub fn is_safe_url(url: &str) -> bool {
+    if let Ok(url) = url.parse::<url::Url>() {
+        if url.host_str().unwrap_or_default() == "unix"
+            || !["http", "https"].contains(&url.scheme())
+            || ![None, Some(80), Some(443)].contains(&url.port())
+        {
+            return false;
+        }
+        true
+    } else {
+        false
+    }
+}
+
+#[cfg(test)]
+mod unit_test {
+    use super::is_safe_url;
+
+    #[test]
+    fn safe_url() {
+        assert!(is_safe_url("http://firefish.dev/firefish/firefish"));
+        assert!(is_safe_url("https://firefish.dev/firefish/firefish"));
+        assert!(is_safe_url("http://firefish.dev:80/firefish/firefish"));
+        assert!(is_safe_url("https://firefish.dev:80/firefish/firefish"));
+        assert!(is_safe_url("http://firefish.dev:443/firefish/firefish"));
+        assert!(is_safe_url("https://firefish.dev:443/firefish/firefish"));
+        assert!(!is_safe_url("https://unix/firefish/firefish"));
+        assert!(!is_safe_url("https://firefish.dev:35/firefish/firefish"));
+        assert!(!is_safe_url("ftp://firefish.dev/firefish/firefish"));
+        assert!(!is_safe_url("nyaa"));
+        assert!(!is_safe_url(""));
+    }
+}

packages/backend-rs/src/misc/mod.rs

@@ -8,6 +8,7 @@ pub mod escape_sql;
 pub mod format_milliseconds;
 pub mod get_image_size;
 pub mod get_note_summary;
+pub mod is_safe_url;
 pub mod latest_version;
 pub mod mastodon_id;
 pub mod meta;

packages/backend/src/misc/download-url.ts

@@ -7,10 +7,10 @@ import chalk from "chalk";
 import Logger from "@/services/logger.js";
 import IPCIDR from "ip-cidr";
 import PrivateIp from "private-ip";
-import { isValidUrl } from "./is-valid-url.js";
+import { isSafeUrl } from "backend-rs";
 
 export async function downloadUrl(url: string, path: string): Promise<void> {
-	if (!isValidUrl(url)) {
+	if (!isSafeUrl(url)) {
 		throw new StatusError("Invalid URL", 400);
 	}
 
@@ -43,8 +43,8 @@ export async function downloadUrl(url: string, path: string): Promise<void> {
 				limit: 0,
 			},
 		})
-		.on("redirect", (res: Got.Response, opts: Got.NormalizedOptions) => {
+		.on("redirect", (_res: Got.Response, opts: Got.NormalizedOptions) => {
-			if (!isValidUrl(opts.url)) {
+			if (!isSafeUrl(opts.url)) {
 				downloadLogger.warn(`Invalid URL: ${opts.url}`);
 				req.destroy();
 			}

packages/backend/src/misc/fetch.ts

@@ -5,7 +5,7 @@ import CacheableLookup from "cacheable-lookup";
 import fetch, { type RequestRedirect } from "node-fetch";
 import { HttpProxyAgent, HttpsProxyAgent } from "hpagent";
 import { config } from "@/config.js";
-import { isValidUrl } from "./is-valid-url.js";
+import { isSafeUrl } from "backend-rs";
 
 export async function getJson(
 	url: string,
@@ -60,7 +60,7 @@ export async function getResponse(args: {
 	size?: number;
 	redirect?: RequestRedirect;
 }) {
-	if (!isValidUrl(args.url)) {
+	if (!isSafeUrl(args.url)) {
 		throw new StatusError("Invalid URL", 400);
 	}
 
@@ -83,7 +83,7 @@ export async function getResponse(args: {
 	});
 
 	if (args.redirect === "manual" && [301, 302, 307, 308].includes(res.status)) {
-		if (!isValidUrl(res.url)) {
+		if (!isSafeUrl(res.url)) {
 			throw new StatusError("Invalid URL", 400);
 		}
 		return res;

packages/backend/src/misc/is-valid-url.ts

@@ -1,20 +0,0 @@
-export function isValidUrl(url: string | URL | undefined): boolean {
-	if (process.env.NODE_ENV !== "production") return true;
-
-	try {
-		if (url == null) return false;
-
-		const u = typeof url === "string" ? new URL(url) : url;
-		if (!u.protocol.match(/^https?:$/) || u.hostname === "unix") {
-			return false;
-		}
-
-		if (u.port !== "" && !["80", "443"].includes(u.port)) {
-			return false;
-		}
-
-		return true;
-	} catch {
-		return false;
-	}
-}

packages/backend/src/remote/activitypub/request.ts

@@ -5,8 +5,8 @@ import { StatusError, getResponse } from "@/misc/fetch.js";
 import { createSignedPost, createSignedGet } from "./ap-request.js";
 import type { Response } from "node-fetch";
 import type { IObject } from "./type.js";
-import { isValidUrl } from "@/misc/is-valid-url.js";
 import { apLogger } from "@/remote/activitypub/logger.js";
+import { isSafeUrl } from "backend-rs";
 
 export default async (user: { id: User["id"] }, url: string, object: any) => {
 	const body = JSON.stringify(object);
@@ -44,7 +44,7 @@ export async function apGet(
 	user?: ILocalUser,
 	redirects: boolean = true,
 ): Promise<{ finalUrl: string; content: IObject }> {
-	if (!isValidUrl(url)) {
+	if (!isSafeUrl(url)) {
 		throw new StatusError("Invalid URL", 400);
 	}