mirror of
https://github.com/Crosstalk-Solutions/project-nomad.git
synced 2026-03-28 03:29:25 +01:00
fix(security): narrow SSRF scope to allow RFC1918 LAN addresses
NOMAD is a LAN appliance — blocking RFC1918 private ranges (10.x, 172.16-31.x, 192.168.x) would prevent users from downloading content from local network mirrors. Narrowed to only block loopback (localhost, 127.x, 0.0.0.0, ::1) and link-local (169.254.x, fe80::) addresses. Restored require_tld: false for LAN hostnames without TLDs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Chris Sherwood
Jake Turner
parent
db1fe84553
commit
20c28cb811
|
|
@ -1,31 +1,31 @@
|
|||
import vine from '@vinejs/vine'
|
||||
|
||||
/**
|
||||
* Checks whether a URL points to a private/internal network address.
|
||||
* Used to prevent SSRF — the server should not fetch from localhost,
|
||||
* private RFC1918 ranges, link-local, or cloud metadata endpoints.
|
||||
* Checks whether a URL points to a loopback or link-local address.
|
||||
* Used to prevent SSRF — the server should not fetch from localhost
|
||||
* or link-local/metadata endpoints (e.g. cloud instance metadata at 169.254.x.x).
|
||||
*
|
||||
* Throws an error if the URL is internal/private.
|
||||
* RFC1918 private ranges (10.x, 172.16-31.x, 192.168.x) are intentionally
|
||||
* ALLOWED because NOMAD is a LAN appliance and users may host content
|
||||
* mirrors on their local network.
|
||||
*
|
||||
* Throws an error if the URL is a loopback or link-local address.
|
||||
*/
|
||||
export function assertNotPrivateUrl(urlString: string): void {
|
||||
const parsed = new URL(urlString)
|
||||
const hostname = parsed.hostname.toLowerCase()
|
||||
|
||||
const privatePatterns = [
|
||||
const blockedPatterns = [
|
||||
/^localhost$/,
|
||||
/^127\.\d+\.\d+\.\d+$/,
|
||||
/^10\.\d+\.\d+\.\d+$/,
|
||||
/^172\.(1[6-9]|2\d|3[01])\.\d+\.\d+$/,
|
||||
/^192\.168\.\d+\.\d+$/,
|
||||
/^169\.254\.\d+\.\d+$/, // Link-local / cloud metadata
|
||||
/^0\.0\.0\.0$/,
|
||||
/^169\.254\.\d+\.\d+$/, // Link-local / cloud metadata
|
||||
/^\[::1\]$/,
|
||||
/^\[?fe80:/i,
|
||||
/^\[?fd[0-9a-f]{2}:/i, // Unique local IPv6
|
||||
/^\[?fe80:/i, // IPv6 link-local
|
||||
]
|
||||
|
||||
if (privatePatterns.some((re) => re.test(hostname))) {
|
||||
throw new Error(`Download URL must not point to a private/internal address: ${hostname}`)
|
||||
if (blockedPatterns.some((re) => re.test(hostname))) {
|
||||
throw new Error(`Download URL must not point to a loopback or link-local address: ${hostname}`)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -33,7 +33,7 @@ export const remoteDownloadValidator = vine.compile(
|
|||
vine.object({
|
||||
url: vine
|
||||
.string()
|
||||
.url()
|
||||
.url({ require_tld: false }) // Allow LAN URLs (e.g. http://my-nas:8080/file.zim)
|
||||
.trim(),
|
||||
})
|
||||
)
|
||||
|
|
@ -42,7 +42,7 @@ export const remoteDownloadWithMetadataValidator = vine.compile(
|
|||
vine.object({
|
||||
url: vine
|
||||
.string()
|
||||
.url()
|
||||
.url({ require_tld: false }) // Allow LAN URLs
|
||||
.trim(),
|
||||
metadata: vine
|
||||
.object({
|
||||
|
|
@ -59,7 +59,7 @@ export const remoteDownloadValidatorOptional = vine.compile(
|
|||
vine.object({
|
||||
url: vine
|
||||
.string()
|
||||
.url()
|
||||
.url({ require_tld: false }) // Allow LAN URLs
|
||||
.trim()
|
||||
.optional(),
|
||||
})
|
||||
|
|
@ -97,7 +97,7 @@ const resourceUpdateInfoBase = vine.object({
|
|||
resource_type: vine.enum(['zim', 'map'] as const),
|
||||
installed_version: vine.string().trim(),
|
||||
latest_version: vine.string().trim().minLength(1),
|
||||
download_url: vine.string().url().trim(),
|
||||
download_url: vine.string().url({ require_tld: false }).trim(),
|
||||
})
|
||||
|
||||
export const applyContentUpdateValidator = vine.compile(resourceUpdateInfoBase)
|
||||
|
|
|
|||
|
|
@ -109,47 +109,29 @@ if (!fullPath.startsWith(path.resolve(basePath))) {
|
|||
|
||||
### 4. SSRF — Download Endpoints (HIGH)
|
||||
|
||||
**File:** `admin/app/validators/common.ts:3-12`
|
||||
**File:** `admin/app/validators/common.ts`
|
||||
**Endpoints:** `POST /api/zim/download-remote`, `POST /api/maps/download-remote`, `POST /api/maps/download-base-assets`, `POST /api/maps/download-remote-preflight`
|
||||
|
||||
The URL validator uses `require_tld: false`, allowing internal/private URLs:
|
||||
|
||||
```typescript
|
||||
url: vine.string().url({ require_tld: false })
|
||||
```
|
||||
|
||||
An attacker on the LAN can make NOMAD fetch from:
|
||||
The download endpoints accept user-supplied URLs and the server fetches from them. Without validation, an attacker on the LAN (or via CSRF since `shield.ts` disables CSRF protection) could make NOMAD fetch from co-located services:
|
||||
- `http://localhost:3306` (MySQL)
|
||||
- `http://localhost:6379` (Redis)
|
||||
- `http://169.254.169.254/` (cloud metadata — if NOMAD is ever cloud-hosted)
|
||||
- Any internal network service
|
||||
|
||||
**Fix:** Add a URL validation helper that rejects private/internal IPs:
|
||||
**Fix:** Added `assertNotPrivateUrl()` that blocks loopback and link-local addresses before any download is initiated. Called in all download controllers.
|
||||
|
||||
**Scope note:** RFC1918 private addresses (10.x, 172.16-31.x, 192.168.x) are intentionally **allowed** because NOMAD is a LAN appliance and users may host content mirrors on their local network. The `require_tld: false` VineJS option is preserved so URLs like `http://my-nas:8080/file.zim` remain valid.
|
||||
|
||||
```typescript
|
||||
function isPrivateUrl(urlString: string): boolean {
|
||||
const url = new URL(urlString)
|
||||
const hostname = url.hostname
|
||||
// Block localhost, private ranges, link-local, metadata
|
||||
const blocked = [
|
||||
/^localhost$/i,
|
||||
/^127\./,
|
||||
/^10\./,
|
||||
/^172\.(1[6-9]|2\d|3[01])\./,
|
||||
/^192\.168\./,
|
||||
/^169\.254\./,
|
||||
/^0\./,
|
||||
/^\[::1\]$/,
|
||||
/^\[fe80:/i,
|
||||
]
|
||||
return blocked.some(re => re.test(hostname))
|
||||
}
|
||||
const blockedPatterns = [
|
||||
/^localhost$/,
|
||||
/^127\.\d+\.\d+\.\d+$/,
|
||||
/^0\.0\.0\.0$/,
|
||||
/^169\.254\.\d+\.\d+$/, // Link-local / cloud metadata
|
||||
/^\[::1\]$/,
|
||||
/^\[?fe80:/i, // IPv6 link-local
|
||||
]
|
||||
```
|
||||
|
||||
Alternatively, maintain an allowlist of known-good download domains (e.g., `download.kiwix.org`, `github.com`, `api.projectnomad.us`).
|
||||
|
||||
**Note:** The `require_tld: false` setting was intentionally added with the comment "Allow local URLs" — but this should only be needed for dev/testing. Consider making it configurable via environment variable, or keeping the allowlist approach for production.
|
||||
|
||||
---
|
||||
|
||||
## FIX AFTER LAUNCH (Medium Priority)
|
||||
|
|
@ -186,7 +168,7 @@ The `updateSetting` endpoint validates the key against an enum, but `getSetting`
|
|||
|
||||
The `download_url` comes directly from the client request body. An attacker can supply any URL and NOMAD will download from it. The URL should be looked up server-side from the content manifest instead.
|
||||
|
||||
**Fix:** Validate `download_url` against the cached manifest, or apply the same SSRF protections as finding #4.
|
||||
**Fix:** Validate `download_url` against the cached manifest, or apply the same loopback/link-local protections as finding #4 (already applied in this PR).
|
||||
|
||||
---
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user