-
Notifications
You must be signed in to change notification settings - Fork 49
fix(evm): validate NFT media/metadata URLs before fetching #1278
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,8 @@ import { ApiError as BlockbookApiError } from '@shapeshiftoss/blockbook' | |
| import { ApiError } from '.' | ||
| import axios, { CreateAxiosDefaults, isAxiosError } from 'axios' | ||
| import axiosRetry, { isNetworkOrIdempotentRequestError } from 'axios-retry' | ||
| import { promises as dns } from 'dns' | ||
| import { isIP } from 'net' | ||
|
|
||
| const MAX_PAGE_SIZE = 100 | ||
|
|
||
|
|
@@ -71,3 +73,54 @@ export const rpcId = (): number => { | |
| if (_rpcId === 0) _rpcId = 1 | ||
| return _rpcId | ||
| } | ||
|
|
||
| const isPrivateIPv4 = (ip: string): boolean => { | ||
| const parts = ip.split('.').map(Number) | ||
| if (parts.length !== 4 || parts.some((n) => Number.isNaN(n) || n < 0 || n > 255)) return true | ||
| const [a, b] = parts | ||
| if (a === 0) return true // 0.0.0.0/8 "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 === 169 && b === 254) return true // 169.254.0.0/16 link-local + cloud 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 === 100 && b >= 64 && b <= 127) return true // 100.64.0.0/10 CGNAT | ||
| if (a >= 224) return true // 224.0.0.0/4 multicast + 240.0.0.0/4 reserved | ||
| return false | ||
| } | ||
|
|
||
| const isPrivateIPv6 = (ip: string): boolean => { | ||
| const v = ip.toLowerCase() | ||
| if (v === '::1' || v === '::') return true // loopback / unspecified | ||
| if (v.startsWith('fe80:')) return true // link-local | ||
| if (v.startsWith('fc') || v.startsWith('fd')) return true // fc00::/7 unique-local | ||
| const mapped = v.match(/^::ffff:(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})$/) // IPv4-mapped | ||
| if (mapped) return isPrivateIPv4(mapped[1]) | ||
| return false | ||
| } | ||
|
|
||
| // assertSafeOutboundUrl validates a caller-influenced URL before the server fetches it, to prevent | ||
| // SSRF: it allows only http(s), resolves the host, and rejects any private/loopback/link-local/CGNAT | ||
| // or cloud-metadata destination. Pair with `maxRedirects: 0` on the request so a public host can't | ||
| // 3xx-bounce to an internal one after this check. | ||
| export const assertSafeOutboundUrl = async (rawUrl: string): Promise<void> => { | ||
| let url: URL | ||
| try { | ||
| url = new URL(rawUrl) | ||
| } catch { | ||
| throw new ApiError('Bad Request', 400, `invalid outbound url: ${rawUrl}`) | ||
| } | ||
|
|
||
| if (url.protocol !== 'https:' && url.protocol !== 'http:') { | ||
| throw new ApiError('Bad Request', 400, `unsupported outbound url scheme: ${url.protocol}`) | ||
| } | ||
|
|
||
| const host = url.hostname.replace(/^\[|\]$/g, '') // strip brackets from IPv6 literals | ||
|
|
||
| const addresses = isIP(host) ? [host] : (await dns.lookup(host, { all: true })).map((r) => r.address) | ||
|
|
||
| for (const address of addresses) { | ||
| const blocked = isIP(address) === 6 ? isPrivateIPv6(address) : isPrivateIPv4(address) | ||
| if (blocked) throw new ApiError('Bad Request', 400, `blocked outbound url host: ${host}`) | ||
| } | ||
| } | ||
|
Comment on lines
+106
to
+126
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
# 1) Locate usages of assertSafeOutboundUrl
rg -n "assertSafeOutboundUrl" -S . || true
# 2) Inspect the referenced files
for f in node/coinstacks/common/api/src/evm/blockbookService.ts node/coinstacks/common/api/src/evm/moralisService.ts node/coinstacks/common/api/src/utils.ts; do
echo "----- $f -----"
if [ -f "$f" ]; then
wc -l "$f"
sed -n '1,220p' "$f" | nl -ba | sed -n '1,220p'
else
echo "Missing: $f"
fi
done
# 3) Search for Axios/URL/agent usage near the call sites
for f in node/coinstacks/common/api/src/evm/blockbookService.ts node/coinstacks/common/api/src/evm/moralisService.ts; do
echo "----- Axios-related in $f -----"
rg -n "axios\.|Axios|http(s)?Agent|dns|lookup|hostname|host:|connect|net\.|lookup:" "$f" -S || true
doneRepository: shapeshift/unchained Length of output: 1319 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Show relevant sections around the call sites
for f in node/coinstacks/common/api/src/utils.ts \
node/coinstacks/common/api/src/evm/blockbookService.ts \
node/coinstacks/common/api/src/evm/moralisService.ts; do
echo "===== $f ====="
wc -l "$f"
done
echo "===== blockbookService.ts: around assertSafeOutboundUrl calls ====="
sed -n '760,920p' node/coinstacks/common/api/src/evm/blockbookService.ts | cat -n
echo "===== moralisService.ts: around assertSafeOutboundUrl calls ====="
sed -n '720,860p' node/coinstacks/common/api/src/evm/moralisService.ts | cat -n
echo "===== utils.ts: assertSafeOutboundUrl ====="
sed -n '70,170p' node/coinstacks/common/api/src/utils.ts | cat -n
echo "===== blockbookService.ts: axios creation/config near top ====="
sed -n '1,120p' node/coinstacks/common/api/src/evm/blockbookService.ts | cat -n
echo "===== moralisService.ts: axios creation/config near top ====="
sed -n '1,120p' node/coinstacks/common/api/src/evm/moralisService.ts | cat -n
# Also find axios invocation arguments near those call sites
echo "===== blockbookService.ts: axios usage ====="
rg -n "axios|createAxiosRetry|http[s]?Agent|lookup|hostname|host:|url:" node/coinstacks/common/api/src/evm/blockbookService.ts -S
echo "===== moralisService.ts: axios usage ====="
rg -n "axios|createAxiosRetry|http[s]?Agent|lookup|hostname|host:|url:" node/coinstacks/common/api/src/evm/moralisService.ts -SRepository: shapeshift/unchained Length of output: 27124 🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n "httpsAgent|httpAgent|agent:|lookup:|createConnection|dns\.lookup|family:" node/coinstacks/common/api/src/evm/blockbookService.ts node/coinstacks/common/api/src/evm/moralisService.ts node/coinstacks/common/api/src/utils.ts -S || true
# Also scan axiosNoRetry.get/head call option objects for agent-related keys
rg -n "axiosNoRetry\.(get|head)\([^)]*,\s*\{[^}]*\}" node/coinstacks/common/api/src/evm/blockbookService.ts node/coinstacks/common/api/src/evm/moralisService.ts -S || trueRepository: shapeshift/unchained Length of output: 776 Bind DNS validation to the outbound connection
🤖 Prompt for AI Agents |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reject hex-form IPv4-mapped IPv6 literals.
Line 97 only catches
::ffff:127.0.0.1-style mapped addresses. Valid literals like::ffff:7f00:1still decode to loopback/private IPv4 targets, butisPrivateIPv6()currently treats them as public, so this guard can still be bypassed with a direct IPv6 literal or AAAA answer.🤖 Prompt for AI Agents