mirror of
https://github.com/makeplane/plane.git
synced 2026-09-01 19:48:42 +02:00
[SECUR-245] fix(security): guard PDF image srcs and authenticate /convert-document/
The Live service exposed two problems:
1. SSRF via PDF image rendering. The `image` node renderer passed
`node.attrs.src` straight to `@react-pdf/image`, which fetch()es any URL
with a host and fs.readFile()s a bare path. Because the Live container
shares a Docker network with api, db, redis, rabbitmq and minio, page
content could drive requests at internal-only services.
Adds `apps/live/src/lib/url-security.ts` and routes both `<Image>` call
sites through it. Unsafe srcs render a placeholder instead.
Note the existing `imageComponent` check was not a usable model: its
`startsWith("http")` test passes `http://api:8000/` and
`http://plane-minio:9000/` — every payload that matters. The scheme is
irrelevant; the destination is what has to be judged. That check is
replaced too.
Blocked: non-http(s)/data schemes, bare and relative filesystem paths,
loopback, RFC1918, CGNAT 100.64/10, link-local incl. 169.254.169.254,
multicast, reserved and test ranges, IPv6 ULA/link-local/site-local/
multicast/NAT64/6to4/Teredo/IPv4-mapped, obfuscated encodings
(2130706433, 0x7f000001, 127.1), single-label hosts (the shape of a
Compose service name), .local/.internal/.lan suffixes, embedded
credentials, and control-character scheme smuggling.
The IPv6 ranges are kept in step with the Python guard's
_BLOCKED_NETWORKS in apps/api/plane/utils/ip_address.py; the first draft
here was missing Teredo and fec0::/10, and two implementations of one
policy drifting apart is how this class of bug keeps recurring.
2. Unauthenticated /convert-document/. `requireSecretKey` existed but was
applied to no controller, leaving an expensive HTML -> Y.js conversion
open to anyone who could reach the service (CWE-306). It is now applied.
Its only caller — the API's copy_s3_object duplication task — sent
`headers=None`, so it now sends the shared secret, and
LIVE_SERVER_SECRET_KEY is wired into Django settings (it was previously
only in .env.example). A missing key short-circuits with a logged
misconfiguration rather than firing a request that can only 401.
Scope notes:
- /pdf-export/ is deliberately untouched. It is not unauthenticated: it
requires a Cookie and forwards it to the API to fetch the page, so the
API enforces page permissions. Gating it on the shared secret would
break the browser client that design implies.
- /convert-document/ performs no outbound fetch, and its output is never
fed to /pdf-export/, which reads content from the API by pageId.
- Residual DNS rebinding on the http(s) path is documented in the helper
and NOT closed here: the renderer is synchronous and the fetch happens
inside @react-pdf/image, so the resolved address cannot be pinned.
Closing it means pre-fetching raw image nodes into data: URIs the way
imageComponent already pre-fetches assets. Follow-up to SECUR-245.
Tests: 78 new Live tests covering every blocked payload plus range
boundaries, and 6 API tests for the header contract and the
missing-key/no-live-url paths.
Co-authored-by: Plane AI <noreply@plane.so>
This commit is contained in:
@@ -77,7 +77,19 @@ def sync_with_external_service(entity_name, description_html):
|
||||
|
||||
url = normalize_url_path(f"{live_url}/convert-document/")
|
||||
|
||||
response = requests.post(url, json=data, headers=None)
|
||||
# The Live service authenticates this endpoint with a shared secret
|
||||
# (GHSA-55gq-rf47-9pqx). Without the header the request is rejected as 401.
|
||||
secret_key = settings.LIVE_SERVER_SECRET_KEY
|
||||
if not secret_key:
|
||||
log_exception(
|
||||
Exception(
|
||||
"LIVE_SERVER_SECRET_KEY is not configured; skipping document conversion "
|
||||
"for duplication. Set it to the same value as the Live service."
|
||||
)
|
||||
)
|
||||
return {}
|
||||
|
||||
response = requests.post(url, json=data, headers={"live-server-secret-key": secret_key})
|
||||
if response.status_code == 200:
|
||||
return response.json()
|
||||
except requests.RequestException as e:
|
||||
|
||||
@@ -418,6 +418,10 @@ LIVE_BASE_PATH = os.environ.get("LIVE_BASE_PATH", "/live/")
|
||||
|
||||
LIVE_URL = urljoin(LIVE_BASE_URL, LIVE_BASE_PATH) if LIVE_BASE_URL else None
|
||||
|
||||
# Shared secret for server-to-server calls into the Live service. Must match the
|
||||
# Live container's LIVE_SERVER_SECRET_KEY.
|
||||
LIVE_SERVER_SECRET_KEY = os.environ.get("LIVE_SERVER_SECRET_KEY")
|
||||
|
||||
# WEB URL
|
||||
WEB_URL = os.environ.get("WEB_URL")
|
||||
|
||||
|
||||
105
apps/api/plane/tests/unit/bg_tasks/test_copy_s3_object_auth.py
Normal file
105
apps/api/plane/tests/unit/bg_tasks/test_copy_s3_object_auth.py
Normal file
@@ -0,0 +1,105 @@
|
||||
# Copyright (c) 2023-present Plane Software, Inc. and contributors
|
||||
# SPDX-License-Identifier: AGPL-3.0-only
|
||||
# See the LICENSE file for details.
|
||||
|
||||
"""
|
||||
Authentication of the API -> Live `/convert-document/` call (GHSA-55gq-rf47-9pqx).
|
||||
|
||||
The Live service previously served `/convert-document/` to anyone who could reach
|
||||
it (`requireSecretKey` was defined but never applied to a controller). Now that the
|
||||
endpoint is gated on the `live-server-secret-key` header, this background task is
|
||||
the one caller that has to present it — so the header must actually be sent, and a
|
||||
missing key must fail loudly rather than firing a request that 401s.
|
||||
|
||||
These are pure unit tests: no database, no network.
|
||||
"""
|
||||
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
from django.test import override_settings
|
||||
|
||||
from plane.bgtasks.copy_s3_object import sync_with_external_service
|
||||
|
||||
LIVE_URL = "http://live:3000/live/"
|
||||
SECRET = "unit-test-live-secret"
|
||||
|
||||
|
||||
@override_settings(LIVE_URL=LIVE_URL, LIVE_SERVER_SECRET_KEY=SECRET)
|
||||
def test_sends_secret_key_header():
|
||||
"""The shared secret must travel on the request, or Live returns 401."""
|
||||
response = MagicMock(status_code=200)
|
||||
response.json.return_value = {"description_json": {}, "description_binary": "AA=="}
|
||||
|
||||
with patch("plane.bgtasks.copy_s3_object.requests.post", return_value=response) as mock_post:
|
||||
result = sync_with_external_service("PAGE", "<p>hello</p>")
|
||||
|
||||
assert result == {"description_json": {}, "description_binary": "AA=="}
|
||||
mock_post.assert_called_once()
|
||||
|
||||
headers = mock_post.call_args.kwargs["headers"]
|
||||
assert headers == {"live-server-secret-key": SECRET}
|
||||
|
||||
|
||||
@override_settings(LIVE_URL=LIVE_URL, LIVE_SERVER_SECRET_KEY=None)
|
||||
def test_missing_secret_key_skips_request():
|
||||
"""
|
||||
With no key configured the call could only ever 401, so it is not attempted.
|
||||
Returning {} leaves `description_binary` untouched upstream (the caller guards
|
||||
on `if external_data:`), which degrades duplication rather than corrupting it.
|
||||
"""
|
||||
with (
|
||||
patch("plane.bgtasks.copy_s3_object.requests.post") as mock_post,
|
||||
patch("plane.bgtasks.copy_s3_object.log_exception") as mock_log,
|
||||
):
|
||||
result = sync_with_external_service("PAGE", "<p>hello</p>")
|
||||
|
||||
assert result == {}
|
||||
mock_post.assert_not_called()
|
||||
# The misconfiguration must be surfaced, not swallowed silently.
|
||||
mock_log.assert_called_once()
|
||||
|
||||
|
||||
@override_settings(LIVE_URL=LIVE_URL, LIVE_SERVER_SECRET_KEY="")
|
||||
def test_empty_secret_key_treated_as_missing():
|
||||
"""An empty string is a misconfiguration, not a valid credential."""
|
||||
with (
|
||||
patch("plane.bgtasks.copy_s3_object.requests.post") as mock_post,
|
||||
patch("plane.bgtasks.copy_s3_object.log_exception"),
|
||||
):
|
||||
result = sync_with_external_service("PAGE", "<p>hello</p>")
|
||||
|
||||
assert result == {}
|
||||
mock_post.assert_not_called()
|
||||
|
||||
|
||||
@override_settings(LIVE_URL=None, LIVE_SERVER_SECRET_KEY=SECRET)
|
||||
def test_no_live_url_short_circuits():
|
||||
"""Deployments without a Live service must not attempt the call at all."""
|
||||
with patch("plane.bgtasks.copy_s3_object.requests.post") as mock_post:
|
||||
result = sync_with_external_service("PAGE", "<p>hello</p>")
|
||||
|
||||
assert result == {}
|
||||
mock_post.assert_not_called()
|
||||
|
||||
|
||||
@override_settings(LIVE_URL=LIVE_URL, LIVE_SERVER_SECRET_KEY=SECRET)
|
||||
def test_non_200_returns_empty_dict():
|
||||
"""A rejected call (e.g. a stale key on one side) must not raise."""
|
||||
with patch("plane.bgtasks.copy_s3_object.requests.post", return_value=MagicMock(status_code=401)):
|
||||
result = sync_with_external_service("PAGE", "<p>hello</p>")
|
||||
|
||||
assert result == {}
|
||||
|
||||
|
||||
@override_settings(LIVE_URL=LIVE_URL, LIVE_SERVER_SECRET_KEY=SECRET)
|
||||
def test_variant_depends_on_entity_name():
|
||||
"""Guard the existing contract while changing the auth around it."""
|
||||
response = MagicMock(status_code=200)
|
||||
response.json.return_value = {}
|
||||
|
||||
with patch("plane.bgtasks.copy_s3_object.requests.post", return_value=response) as mock_post:
|
||||
sync_with_external_service("PAGE", "<p>x</p>")
|
||||
assert mock_post.call_args.kwargs["json"]["variant"] == "rich"
|
||||
|
||||
sync_with_external_service("ISSUE", "<p>x</p>")
|
||||
assert mock_post.call_args.kwargs["json"]["variant"] == "document"
|
||||
@@ -7,10 +7,11 @@
|
||||
import type { Request, Response } from "express";
|
||||
import { z } from "zod";
|
||||
// helpers
|
||||
import { Controller, Post } from "@plane/decorators";
|
||||
import { Controller, Middleware, Post } from "@plane/decorators";
|
||||
import { convertHTMLDocumentToAllFormats } from "@plane/editor";
|
||||
// logger
|
||||
import { logger } from "@plane/logger";
|
||||
import { requireSecretKey } from "@/lib/auth-middleware";
|
||||
import type { TConvertDocumentRequestBody } from "@/types";
|
||||
|
||||
// Define the schema with more robust validation
|
||||
@@ -25,7 +26,15 @@ const convertDocumentSchema = z.object({
|
||||
|
||||
@Controller("/convert-document")
|
||||
export class DocumentController {
|
||||
/**
|
||||
* Server-to-server only: the sole caller is the API's `copy_s3_object` background
|
||||
* task (page / work-item duplication). It was previously reachable unauthenticated
|
||||
* by anyone who could hit the Live service, which made an expensive HTML -> Y.js
|
||||
* conversion available as free compute to the internet (GHSA-55gq-rf47-9pqx,
|
||||
* CWE-306). Callers must now present `live-server-secret-key`.
|
||||
*/
|
||||
@Post("/")
|
||||
@Middleware(requireSecretKey)
|
||||
async convertDocument(req: Request, res: Response) {
|
||||
try {
|
||||
// Validate request body
|
||||
|
||||
@@ -8,6 +8,7 @@ import { Image, Link, Text, View } from "@react-pdf/renderer";
|
||||
import type { Style } from "@react-pdf/types";
|
||||
import type { ReactElement } from "react";
|
||||
import { CORE_EXTENSIONS } from "@plane/editor";
|
||||
import { isSafeImageSrc } from "@/lib/url-security";
|
||||
import { BACKGROUND_COLORS, EDITOR_BACKGROUND_COLORS, resolveColorForPdf, TEXT_COLORS } from "./colors";
|
||||
import { CheckIcon, ClipboardIcon, DocumentIcon, GlobeIcon, LightbulbIcon, LinkIcon } from "./icons";
|
||||
import { applyMarks } from "./mark-renderers";
|
||||
@@ -272,6 +273,18 @@ export const nodeRenderers: NodeRendererRegistry = {
|
||||
? { alignItems: "flex-end" as const }
|
||||
: { alignItems: "flex-start" as const };
|
||||
|
||||
// SSRF guard (GHSA-55gq-rf47-9pqx). `src` comes from page content, and
|
||||
// @react-pdf/image will fetch() any URL with a host — including internal
|
||||
// Docker service names — or fs.readFile() a bare path. Anything we are not
|
||||
// willing to fetch renders as a placeholder instead.
|
||||
if (!isSafeImageSrc(src)) {
|
||||
return (
|
||||
<View key={ctx.getKey()} style={[pdfStyles.imagePlaceholder, alignmentStyle]}>
|
||||
<Text style={pdfStyles.imagePlaceholderText}>[Image unavailable]</Text>
|
||||
</View>
|
||||
);
|
||||
}
|
||||
|
||||
return (
|
||||
<View key={ctx.getKey()} style={[{ width: "100%" }, alignmentStyle]}>
|
||||
<Image
|
||||
@@ -308,7 +321,12 @@ export const nodeRenderers: NodeRendererRegistry = {
|
||||
? { alignItems: "flex-end" as const }
|
||||
: { alignItems: "flex-start" as const };
|
||||
|
||||
if (!resolvedSrc.startsWith("http") && !resolvedSrc.startsWith("data:")) {
|
||||
// Normally `resolvedSrc` is the `data:image/jpeg;base64,…` URI produced by the
|
||||
// service's own pre-fetch, so nothing is fetched at render time. If asset
|
||||
// resolution failed it is still the raw asset id, which is not a fetchable URL.
|
||||
// Use the same guard as the `image` renderer rather than a startsWith("http")
|
||||
// check, which would happily pass http://api:8000/ (GHSA-55gq-rf47-9pqx).
|
||||
if (!isSafeImageSrc(resolvedSrc)) {
|
||||
return (
|
||||
<View key={ctx.getKey()} style={[pdfStyles.imagePlaceholder, alignmentStyle]}>
|
||||
<Text style={pdfStyles.imagePlaceholderText}>[Image: {assetId.slice(0, 8)}...]</Text>
|
||||
|
||||
180
apps/live/src/lib/url-security.ts
Normal file
180
apps/live/src/lib/url-security.ts
Normal file
@@ -0,0 +1,180 @@
|
||||
/**
|
||||
* Copyright (c) 2023-present Plane Software, Inc. and contributors
|
||||
* SPDX-License-Identifier: AGPL-3.0-only
|
||||
* See the LICENSE file for details.
|
||||
*/
|
||||
|
||||
import net from "node:net";
|
||||
|
||||
/**
|
||||
* SSRF guards for URLs that the Live service may cause to be fetched.
|
||||
*
|
||||
* Context (GHSA-55gq-rf47-9pqx): the PDF exporter renders TipTap `image` nodes by
|
||||
* handing `node.attrs.src` straight to `@react-pdf/image`, which calls `fetch()` on
|
||||
* anything with a host. Because the Live container shares a Docker network with the
|
||||
* API, database, Redis, RabbitMQ and MinIO, an unvalidated `src` turns PDF export
|
||||
* into a request forgery primitive against internal-only services.
|
||||
*
|
||||
* A prefix check such as `src.startsWith("http")` does NOT close this: the payloads
|
||||
* that matter — `http://api:8000/`, `http://plane-minio:9000/` — all start with
|
||||
* "http". The scheme is irrelevant; the *destination* is what has to be judged.
|
||||
*/
|
||||
|
||||
/** Schemes we are willing to hand to the PDF image pipeline. */
|
||||
const ALLOWED_SCHEMES = new Set(["http:", "https:", "data:"]);
|
||||
|
||||
/**
|
||||
* Hostname suffixes that only ever resolve inside a private network.
|
||||
* Compared against the lowercased hostname, with a leading dot to avoid
|
||||
* matching a public registrable domain that merely ends in these letters.
|
||||
*/
|
||||
const BLOCKED_HOST_SUFFIXES = [".local", ".localhost", ".internal", ".home.arpa", ".lan"];
|
||||
|
||||
/** Bare hostnames that need no DNS to be dangerous. */
|
||||
const BLOCKED_HOST_EXACT = new Set(["localhost", "metadata", "metadata.google.internal"]);
|
||||
|
||||
/**
|
||||
* Returns true when an IPv4 literal falls in a range that must never be fetched.
|
||||
* Ranges follow IANA special-purpose registries rather than a hand-rolled
|
||||
* "private IP" list, so CGNAT and benchmarking space are covered too.
|
||||
*/
|
||||
const isBlockedIPv4 = (ip: string): boolean => {
|
||||
const parts = ip.split(".").map((p) => Number(p));
|
||||
if (parts.length !== 4 || parts.some((p) => !Number.isInteger(p) || p < 0 || p > 255)) {
|
||||
// Not a canonical dotted quad — callers treat unparseable hosts as unsafe.
|
||||
return true;
|
||||
}
|
||||
const [a, b] = parts as [number, number, number, number];
|
||||
|
||||
if (a === 0) return true; // 0.0.0.0/8 "this host on this network"
|
||||
if (a === 10) return true; // 10.0.0.0/8 private
|
||||
if (a === 127) return true; // 127.0.0.0/8 loopback
|
||||
if (a === 100 && b >= 64 && b <= 127) return true; // 100.64.0.0/10 CGNAT
|
||||
if (a === 169 && b === 254) return true; // 169.254.0.0/16 link-local (incl. 169.254.169.254 metadata)
|
||||
if (a === 172 && b >= 16 && b <= 31) return true; // 172.16.0.0/12 private
|
||||
if (a === 192 && b === 168) return true; // 192.168.0.0/16 private
|
||||
if (a === 192 && b === 0) return true; // 192.0.0.0/24 + 192.0.2.0/24 (IETF protocol / TEST-NET-1)
|
||||
if (a === 198 && (b === 18 || b === 19)) return true; // 198.18.0.0/15 benchmarking
|
||||
if (a === 198 && b === 51) return true; // 198.51.100.0/24 TEST-NET-2
|
||||
if (a === 203 && b === 0) return true; // 203.0.113.0/24 TEST-NET-3
|
||||
if (a >= 224) return true; // 224.0.0.0/4 multicast, 240.0.0.0/4 reserved, 255.255.255.255
|
||||
|
||||
return false;
|
||||
};
|
||||
|
||||
/** Returns true when an IPv6 literal must never be fetched. */
|
||||
const isBlockedIPv6 = (ip: string): boolean => {
|
||||
const addr = ip.toLowerCase().replace(/^\[|\]$/g, "");
|
||||
|
||||
// IPv4-mapped (::ffff:127.0.0.1) and IPv4-compatible forms: judge the embedded v4.
|
||||
const mapped = addr.match(/^(?:::ffff:|::)((?:\d{1,3}\.){3}\d{1,3})$/);
|
||||
if (mapped?.[1]) return isBlockedIPv4(mapped[1]);
|
||||
|
||||
if (addr === "::" || addr === "::1") return true; // unspecified / loopback
|
||||
if (addr.startsWith("fe8") || addr.startsWith("fe9") || addr.startsWith("fea") || addr.startsWith("feb")) return true; // fe80::/10 link-local
|
||||
// fec0::/10 deprecated site-local. Kept in step with the Python guard's
|
||||
// _BLOCKED_NETWORKS (apps/api/plane/utils/ip_address.py) — the two lists must not
|
||||
// drift, or one service will block a range the other happily fetches.
|
||||
if (addr.startsWith("fec") || addr.startsWith("fed") || addr.startsWith("fee") || addr.startsWith("fef")) return true;
|
||||
if (addr.startsWith("fc") || addr.startsWith("fd")) return true; // fc00::/7 unique local
|
||||
if (addr.startsWith("ff")) return true; // ff00::/8 multicast
|
||||
if (addr.startsWith("64:ff9b:")) return true; // 64:ff9b::/96 + 64:ff9b:1::/48 NAT64
|
||||
if (addr.startsWith("2002:")) return true; // 6to4 — can wrap a private v4
|
||||
if (/^2001:(0{1,4})?:/.test(addr)) return true; // 2001::/32 Teredo
|
||||
if (addr.startsWith("::ffff:")) return true; // any other IPv4-mapped form
|
||||
|
||||
return false;
|
||||
};
|
||||
|
||||
/**
|
||||
* Returns true when `host` is an IP literal pointing somewhere we refuse to fetch,
|
||||
* or a numeric/obfuscated host form that is not a canonical address at all.
|
||||
*
|
||||
* Obfuscated encodings (`0x7f000001`, `2130706433`, `127.1`) are rejected outright:
|
||||
* some HTTP clients expand them to loopback, none of them are legitimate image
|
||||
* hosts, and normalising every variant is a losing game.
|
||||
*/
|
||||
export const isBlockedHostLiteral = (host: string): boolean => {
|
||||
const bare = host.replace(/^\[|\]$/g, "");
|
||||
|
||||
const ipVersion = net.isIP(bare);
|
||||
if (ipVersion === 4) return isBlockedIPv4(bare);
|
||||
if (ipVersion === 6) return isBlockedIPv6(bare);
|
||||
|
||||
// Hex (0x…), octal-ish, decimal, or short-form dotted numbers — never a real host.
|
||||
if (/^0x[0-9a-f]+$/i.test(bare)) return true;
|
||||
if (/^[0-9]+$/.test(bare)) return true;
|
||||
if (/^[0-9.]+$/.test(bare)) return true;
|
||||
|
||||
return false;
|
||||
};
|
||||
|
||||
/**
|
||||
* Returns true when a hostname is safe enough to hand to the image fetcher.
|
||||
*
|
||||
* Single-label hostnames are refused because that is exactly the shape of a Docker
|
||||
* Compose service name — `api`, `web`, `plane-db`, `plane-redis`, `plane-minio` —
|
||||
* which is the primary escalation path in this advisory. Public image hosts always
|
||||
* carry a dot.
|
||||
*/
|
||||
const isAllowedHostname = (hostname: string): boolean => {
|
||||
const host = hostname.toLowerCase();
|
||||
|
||||
if (!host) return false;
|
||||
if (BLOCKED_HOST_EXACT.has(host)) return false;
|
||||
if (BLOCKED_HOST_SUFFIXES.some((suffix) => host.endsWith(suffix))) return false;
|
||||
if (isBlockedHostLiteral(host)) return false;
|
||||
// No dot => single-label => container/service name on the internal network.
|
||||
if (!host.includes(".")) return false;
|
||||
// A trailing dot ("api.") sidesteps the check above without adding a real label.
|
||||
if (host.endsWith(".")) return false;
|
||||
|
||||
return true;
|
||||
};
|
||||
|
||||
/**
|
||||
* Decides whether a TipTap image `src` may be passed to the PDF image pipeline.
|
||||
*
|
||||
* `data:` URIs are allowed because the asset pipeline deliberately pre-fetches
|
||||
* images server-side and inlines them as `data:image/jpeg;base64,…`; those never
|
||||
* touch the network again at render time.
|
||||
*
|
||||
* NOTE ON DNS REBINDING: for an `http(s)` host that clears these checks we cannot
|
||||
* pin the resolved address here — the renderer is synchronous and the actual
|
||||
* `fetch()` happens inside `@react-pdf/image`, out of our reach. A hostname under
|
||||
* attacker control that resolves to a blocked address therefore remains a residual
|
||||
* TOCTOU. Closing it properly means pre-fetching raw image nodes the way
|
||||
* `imageComponent` already pre-fetches assets, then rendering only `data:` URIs.
|
||||
* Tracked as follow-up to SECUR-245 — do not mistake this helper for a complete
|
||||
* SSRF defence on the http(s) path.
|
||||
*/
|
||||
export const isSafeImageSrc = (src: string): boolean => {
|
||||
if (!src) return false;
|
||||
|
||||
const trimmed = src.trim();
|
||||
if (!trimmed) return false;
|
||||
|
||||
// Reject control characters and whitespace, which URL parsers strip and
|
||||
// which have historically been used to smuggle a scheme past naive checks.
|
||||
// oxlint-disable-next-line no-control-regex -- intentional: these are exactly what we reject
|
||||
if (/[\u0000-\u0020\u007F]/.test(trimmed)) return false;
|
||||
|
||||
let parsed: URL;
|
||||
try {
|
||||
parsed = new URL(trimmed);
|
||||
} catch {
|
||||
// Relative paths and bare filesystem paths land here. `@react-pdf/image`
|
||||
// would hand those to fs.readFile(), so they are refused.
|
||||
return false;
|
||||
}
|
||||
|
||||
if (!ALLOWED_SCHEMES.has(parsed.protocol)) return false;
|
||||
|
||||
// data: carries its payload inline; there is no host to judge.
|
||||
if (parsed.protocol === "data:") return trimmed.toLowerCase().startsWith("data:image/");
|
||||
|
||||
// Credentials in an image URL are never legitimate and can confuse host parsing.
|
||||
if (parsed.username || parsed.password) return false;
|
||||
|
||||
return isAllowedHostname(parsed.hostname);
|
||||
};
|
||||
201
apps/live/tests/lib/url-security.test.ts
Normal file
201
apps/live/tests/lib/url-security.test.ts
Normal file
@@ -0,0 +1,201 @@
|
||||
/**
|
||||
* Copyright (c) 2023-present Plane Software, Inc. and contributors
|
||||
* SPDX-License-Identifier: AGPL-3.0-only
|
||||
* See the LICENSE file for details.
|
||||
*/
|
||||
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { isBlockedHostLiteral, isSafeImageSrc } from "@/lib/url-security";
|
||||
|
||||
describe("isSafeImageSrc — GHSA-55gq-rf47-9pqx", () => {
|
||||
describe("advisory payloads: internal Docker service names", () => {
|
||||
// The escalation path named in the advisory. Every one of these starts with
|
||||
// "http", which is why the imageComponent-style startsWith("http") guard
|
||||
// does not close this vulnerability.
|
||||
it.each([
|
||||
"http://api:8000/api/workspaces/",
|
||||
"http://plane-minio:9000/uploads/",
|
||||
"http://plane-db:5432/",
|
||||
"http://plane-redis:6379/",
|
||||
"http://plane-mq:5672/",
|
||||
"http://web:3000/",
|
||||
"http://admin:3000/",
|
||||
"http://space:3000/",
|
||||
"http://live:3000/",
|
||||
])("rejects %s", (src) => {
|
||||
expect(isSafeImageSrc(src)).toBe(false);
|
||||
});
|
||||
|
||||
it("rejects a single-label host even over https", () => {
|
||||
expect(isSafeImageSrc("https://api/")).toBe(false);
|
||||
});
|
||||
|
||||
it("rejects a trailing-dot host that would otherwise look multi-label", () => {
|
||||
expect(isSafeImageSrc("http://api./")).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("cloud metadata and loopback", () => {
|
||||
it.each([
|
||||
"http://169.254.169.254/latest/meta-data/",
|
||||
"http://metadata.google.internal/computeMetadata/v1/",
|
||||
"http://metadata/computeMetadata/v1/",
|
||||
"http://localhost:8000/",
|
||||
"http://127.0.0.1:8000/",
|
||||
"http://127.1.2.3/",
|
||||
"http://[::1]/",
|
||||
"http://[::ffff:127.0.0.1]/",
|
||||
])("rejects %s", (src) => {
|
||||
expect(isSafeImageSrc(src)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("private, CGNAT, link-local and reserved ranges", () => {
|
||||
it.each([
|
||||
"http://10.0.0.5/",
|
||||
"http://172.16.0.1/",
|
||||
"http://172.31.255.254/",
|
||||
"http://192.168.1.1/",
|
||||
"http://100.64.0.1/", // CGNAT — missed by naive "private IP" lists
|
||||
"http://100.127.255.255/",
|
||||
"http://0.0.0.0/",
|
||||
"http://224.0.0.1/", // multicast
|
||||
"http://255.255.255.255/",
|
||||
"http://198.18.0.1/", // benchmarking
|
||||
"http://[fd00::1]/", // IPv6 unique local
|
||||
"http://[fe80::1]/", // IPv6 link-local
|
||||
"http://[ff02::1]/", // IPv6 multicast
|
||||
"http://[fec0::1]/", // IPv6 deprecated site-local
|
||||
"http://[2002:7f00:1::]/", // 6to4 wrapping 127.0.0.1
|
||||
"http://[2001::1]/", // Teredo
|
||||
"http://[64:ff9b::7f00:1]/", // NAT64 wrapping 127.0.0.1
|
||||
"http://[64:ff9b:1::1]/", // NAT64 local-use prefix
|
||||
])("rejects %s", (src) => {
|
||||
expect(isSafeImageSrc(src)).toBe(false);
|
||||
});
|
||||
|
||||
it("allows a public IP just outside a blocked range", () => {
|
||||
// 100.63.x is public; the CGNAT block starts at 100.64.
|
||||
expect(isSafeImageSrc("http://100.63.0.1/")).toBe(true);
|
||||
// 172.32.x is public; the private block ends at 172.31.
|
||||
expect(isSafeImageSrc("http://172.32.0.1/")).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("obfuscated address encodings", () => {
|
||||
// Not canonical addresses, but several HTTP clients expand them to loopback.
|
||||
it.each([
|
||||
"http://2130706433/", // decimal 127.0.0.1
|
||||
"http://0x7f000001/", // hex 127.0.0.1
|
||||
"http://127.1/", // short-form 127.0.0.1
|
||||
"http://0/", // shorthand for 0.0.0.0
|
||||
])("rejects %s", (src) => {
|
||||
expect(isSafeImageSrc(src)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("scheme handling", () => {
|
||||
it.each([
|
||||
"file:///etc/passwd",
|
||||
"ftp://example.com/x.png",
|
||||
"gopher://example.com/",
|
||||
"javascript:alert(1)",
|
||||
"vbscript:msgbox(1)",
|
||||
"blob:https://example.com/abc",
|
||||
])("rejects %s", (src) => {
|
||||
expect(isSafeImageSrc(src)).toBe(false);
|
||||
});
|
||||
|
||||
it("rejects bare filesystem paths that would reach fs.readFile", () => {
|
||||
// The advisory's secondary local-file-read finding.
|
||||
expect(isSafeImageSrc("/etc/passwd")).toBe(false);
|
||||
expect(isSafeImageSrc("./relative.png")).toBe(false);
|
||||
expect(isSafeImageSrc("../../etc/hosts")).toBe(false);
|
||||
});
|
||||
|
||||
it("allows image data URIs (the asset pipeline's own output)", () => {
|
||||
expect(isSafeImageSrc("data:image/jpeg;base64,/9j/4AAQSkZJRg==")).toBe(true);
|
||||
expect(isSafeImageSrc("data:image/png;base64,iVBORw0KGgo=")).toBe(true);
|
||||
});
|
||||
|
||||
it("rejects non-image data URIs", () => {
|
||||
expect(isSafeImageSrc("data:text/html,<script>alert(1)</script>")).toBe(false);
|
||||
expect(isSafeImageSrc("data:application/javascript,alert(1)")).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("whitespace and control-character smuggling", () => {
|
||||
// The same class of bypass as GHSA-v2vv-7wq3-8w2j: URL parsers strip these,
|
||||
// so a check performed before stripping can be walked straight past.
|
||||
it.each([
|
||||
"\thttp://api:8000/",
|
||||
"\nhttp://api:8000/",
|
||||
"\rhttp://api:8000/",
|
||||
" http://127.0.0.1/",
|
||||
"http://api\t:8000/",
|
||||
"\u0000http://api:8000/",
|
||||
"\u00A0http://api:8000/", // non-breaking space
|
||||
"\uFEFFhttp://api:8000/", // BOM
|
||||
])("rejects %j", (src) => {
|
||||
expect(isSafeImageSrc(src)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("embedded credentials", () => {
|
||||
it("rejects URLs carrying credentials", () => {
|
||||
expect(isSafeImageSrc("http://user:pass@images.example.com/a.png")).toBe(false);
|
||||
// Credentials can also be used to make the real host hard to read.
|
||||
expect(isSafeImageSrc("http://images.example.com@127.0.0.1/a.png")).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("internal-only hostname suffixes", () => {
|
||||
it.each([
|
||||
"http://printer.local/x.png",
|
||||
"http://app.localhost/x.png",
|
||||
"http://svc.internal/x.png",
|
||||
"http://box.lan/x.png",
|
||||
"http://thing.home.arpa/x.png",
|
||||
])("rejects %s", (src) => {
|
||||
expect(isSafeImageSrc(src)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("legitimate images still render", () => {
|
||||
it.each([
|
||||
"https://images.example.com/photo.png",
|
||||
"http://cdn.example.org/a/b/c.jpg",
|
||||
"https://user-images.githubusercontent.com/1/2.png",
|
||||
"https://example.co.uk/img.webp",
|
||||
"https://sub.domain.example.com:8443/img.png",
|
||||
])("allows %s", (src) => {
|
||||
expect(isSafeImageSrc(src)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("empty and malformed input", () => {
|
||||
it.each(["", " ", "not a url", "http://", "://example.com"])("rejects %j", (src) => {
|
||||
expect(isSafeImageSrc(src)).toBe(false);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("isBlockedHostLiteral", () => {
|
||||
it("classifies canonical IPv4 literals", () => {
|
||||
expect(isBlockedHostLiteral("127.0.0.1")).toBe(true);
|
||||
expect(isBlockedHostLiteral("10.1.2.3")).toBe(true);
|
||||
expect(isBlockedHostLiteral("8.8.8.8")).toBe(false);
|
||||
expect(isBlockedHostLiteral("1.1.1.1")).toBe(false);
|
||||
});
|
||||
|
||||
it("classifies IPv6 literals with and without brackets", () => {
|
||||
expect(isBlockedHostLiteral("::1")).toBe(true);
|
||||
expect(isBlockedHostLiteral("[::1]")).toBe(true);
|
||||
expect(isBlockedHostLiteral("2606:4700:4700::1111")).toBe(false);
|
||||
});
|
||||
|
||||
it("treats real hostnames as non-literals", () => {
|
||||
expect(isBlockedHostLiteral("example.com")).toBe(false);
|
||||
expect(isBlockedHostLiteral("api")).toBe(false);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user