Skip to content

Fix: Fix DNS Rebinding/TOCTOU Vulernability #5653

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.

Sign up for GitHub

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

Open
christopherholland-workday wants to merge 9 commits into
base: main
Choose a base branch
from
+107 −90
Open

Fix: Fix DNS Rebinding/TOCTOU Vulernability #5653

Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
848ef2f
Fix: Fix DNS Rebinding/TOCTOU Vulernability
christopherholland-workday Jan 20, 2026
a554565
Update packages/components/src/httpSecurity.ts
christopherholland-workday Jan 20, 2026
578da56
Update packages/components/src/httpSecurity.ts
christopherholland-workday Jan 20, 2026
f12b6ee
Update packages/components/src/httpSecurity.ts
christopherholland-workday Jan 20, 2026
4d41232
Update httpSecurity.ts
christopherholland-workday Jan 20, 2026
bb05e72
Merge branch 'feature/fix-toctou-bug' of https://github.com/FlowiseAI…
christopherholland-workday Jan 20, 2026
14f1d72
Update httpSecurity.ts
christopherholland-workday Jan 20, 2026
8dda5a1
Update httpSecurity.ts
christopherholland-workday Jan 20, 2026
73b5c24
Update httpSecurity.ts
christopherholland-workday Jan 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
197 changes: 107 additions & 90 deletions packages/components/src/httpSecurity.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
import dns from 'dns/promises'
import axios, { AxiosRequestConfig, AxiosResponse } from 'axios'
import fetch, { RequestInit, Response } from 'node-fetch'
import http from 'http'
import https from 'https'

/**
* Checks if an IP address is in the deny list
Expand Down Expand Up @@ -62,96 +64,58 @@
*/
export async function secureAxiosRequest(config: AxiosRequestConfig, maxRedirects: number = 5): Promise<AxiosResponse> {
let currentUrl = config.url
let redirectCount = 0
let currentConfig = { ...config, maxRedirects: 0 } // Disable automatic redirects

// Validate the initial URL
if (currentUrl) {
await checkDenyList(currentUrl)
if (!currentUrl) {
throw new Error('secureAxiosRequest: url is required')
}

while (redirectCount <= maxRedirects) {
try {
// Update the URL in config for subsequent requests
currentConfig.url = currentUrl

const response = await axios(currentConfig)

// If it's a successful response (not a redirect), return it
if (response.status < 300 || response.status >= 400) {
return response
}

// Handle redirect
const location = response.headers.location
if (!location) {
// No location header, but it's a redirect status - return the response
return response
}

redirectCount++
let redirects = 0
let currentConfig = { ...config, maxRedirects: 0 } // Disable automatic redirects

if (redirectCount > maxRedirects) {
throw new Error('Too many redirects')
while (redirects <= maxRedirects) {
const target = await resolveAndValidate(currentUrl)
const agent = createPinnedAgent(target)

currentConfig = {
...currentConfig,
url: currentUrl,
...(target.protocol === 'http' ? { httpAgent: agent } : { httpsAgent: agent }),
headers: {
...currentConfig.headers,
Host: target.hostname
}
}

// Resolve the redirect URL (handle relative URLs)
const redirectUrl = new URL(location, currentUrl).toString()

// Validate the redirect URL against the deny list
await checkDenyList(redirectUrl)
const response = await axios(currentConfig)

// Update current URL for next iteration
currentUrl = redirectUrl
// If it's a successful response (not a redirect), return it
if (response.status < 300 || response.status >= 400) {
return response
}

// For redirects, we only need to preserve certain headers and change method if needed
if (response.status === 301 || response.status === 302 || response.status === 303) {
// For 303, or when redirecting POST requests, change to GET
if (
response.status === 303 ||
(currentConfig.method && ['POST', 'PUT', 'PATCH'].includes(currentConfig.method.toUpperCase()))
) {
currentConfig.method = 'GET'
delete currentConfig.data
}
}
} catch (error) {
// If it's not a redirect-related error from axios, propagate it
if (error.response && error.response.status >= 300 && error.response.status < 400) {
// This is a redirect response that axios couldn't handle automatically
// Continue with our manual redirect handling
const response = error.response
const location = response.headers.location

if (!location) {
return response
}
// Handle redirect
const location = response.headers.location
if (!location) {
// No location header, but it's a redirect status - return the response
return response
}

redirectCount++
redirects++
if (redirects > maxRedirects) {
throw new Error('Too many redirects')
}

if (redirectCount > maxRedirects) {
throw new Error('Too many redirects')
}
currentUrl = new URL(location, currentUrl).toString()

const redirectUrl = new URL(location, currentUrl).toString()
await checkDenyList(redirectUrl)
currentUrl = redirectUrl

// Handle method changes for redirects
if (response.status === 301 || response.status === 302 || response.status === 303) {
if (
response.status === 303 ||
(currentConfig.method && ['POST', 'PUT', 'PATCH'].includes(currentConfig.method.toUpperCase()))
) {
currentConfig.method = 'GET'
delete currentConfig.data
}
}
continue
// For redirects, we only need to preserve certain headers and change method if needed
if (response.status === 301 || response.status === 302 || response.status === 303) {
// For 303, or when redirecting POST requests, change to GET
if (
response.status === 303 ||
(currentConfig.method && ['POST', 'PUT', 'PATCH'].includes(currentConfig.method.toUpperCase()))
) {
currentConfig.method = 'GET'
delete currentConfig.data
}

// For other errors, re-throw
throw error
}
}

Expand All @@ -171,11 +135,11 @@
let redirectCount = 0
let currentInit = { ...init, redirect: 'manual' as const } // Disable automatic redirects

// Validate the initial URL
await checkDenyList(currentUrl)

while (redirectCount <= maxRedirects) {
const response = await fetch(currentUrl, currentInit)
const resolved = await resolveAndValidate(currentUrl)
const agent = createPinnedAgent(resolved)

const response = await fetch(currentUrl, { ...currentInit, agent: () => agent })

Check failure

Code scanning / CodeQL

Server-side request forgery Critical

The
URL
Loading
of this request depends on a
user-provided value
Loading
.

Copilot Autofix

AI about 16 hours ago

In general, to fix this kind of SSRF issue you ensure that user-controlled input cannot directly choose an arbitrary request target. Instead, you validate or transform the input into a safe value: enforce allowed schemes (typically http/https only), disallow raw IPs if appropriate, and restrict hostnames to an allow-list or at least block link-local/metadata endpoints and private networks regardless of environment configuration. You should also ensure these checks are applied consistently wherever user-controlled URLs are used.

For this codebase, the best minimal fix without changing existing functionality is to strengthen checkDenyList in packages/components/src/httpSecurity.ts so it always enforces baseline scheme and host/IP restrictions, even if HTTP_DENY_LIST is not set. All user-controlled URLs in this flow go through checkDenyList already (fetch-links service calls it explicitly; xmlScrape calls secureFetch, which uses resolveAndValidate internally), so augmenting checkDenyList to also reject non-HTTP(S) schemes and clearly dangerous hostnames/IPs will ensure secureFetch cannot be used for SSRF against internal/metadata endpoints. Concretely:

  • Parse the URL and immediately reject:
    • Non-http/https schemes.
    • Hostnames like localhost, 127.0.0.1, ::1, and well-known cloud metadata hostnames (169.254.169.254, metadata.google.internal, etc.).
  • Maintain the existing deny-list behavior, but apply it in addition to the new static rules.
  • To avoid code duplication and future errors, introduce a small helper (e.g. isDeniedHostname) at the top of checkDenyList to encapsulate these checks before doing DNS resolution.

This keeps the public API the same (checkDenyList signature unchanged) and requires no changes to the callers. All edits are confined to the checkDenyList function in packages/components/src/httpSecurity.ts; no new imports are needed because we reuse URL and existing ipaddr/dns imports.

Suggested changeset 1
packages/components/src/httpSecurity.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/components/src/httpSecurity.ts b/packages/components/src/httpSecurity.ts
--- a/packages/components/src/httpSecurity.ts
+++ b/packages/components/src/httpSecurity.ts
@@ -34,16 +34,45 @@
 
 /**
  * Checks if a URL is allowed based on HTTP_DENY_LIST environment variable
+ * and built-in SSRF protections (scheme and hostname/IP restrictions).
  * @param url - URL to check
- * @throws Error if URL hostname resolves to a denied IP
+ * @throws Error if URL hostname or resolved IP is denied
  */
 export async function checkDenyList(url: string): Promise<void> {
+    const urlObj = new URL(url)
+    const hostname = urlObj.hostname
+    const protocol = urlObj.protocol.toLowerCase()
+
+    // Enforce allowed protocols
+    if (protocol !== 'http:' && protocol !== 'https:') {
+        throw new Error(`Access to this URL is denied by policy: unsupported protocol "${protocol}"`)
+    }
+
+    // Block obvious local/metadata hosts regardless of HTTP_DENY_LIST
+    const loweredHost = hostname.toLowerCase()
+    const blockedHostnames = new Set<string>([
+        'localhost',
+        '127.0.0.1',
+        '::1',
+        '169.254.169.254',          // AWS/GCP metadata IP
+        'metadata.google.internal', // GCP metadata hostname
+        '169.254.169.250',
+        '169.254.170.2'             // AWS ECS task metadata
+    ])
+    if (blockedHostnames.has(loweredHost)) {
+        throw new Error('Access to this host is denied by policy.')
+    }
+
     const httpDenyListString: string | undefined = process.env.HTTP_DENY_LIST
-    if (!httpDenyListString) return
+    if (!httpDenyListString) {
+        // Even if no explicit deny list is configured, still prevent direct IP access
+        if (ipaddr.isValid(hostname)) {
+            throw new Error('Access to this host is denied by policy.')
+        }
+        return
+    }
 
     const httpDenyList = httpDenyListString.split(',').map((ip) => ip.trim())
-    const urlObj = new URL(url)
-    const hostname = urlObj.hostname
 
     if (ipaddr.isValid(hostname)) {
         isDeniedIP(hostname, httpDenyList)
EOF
@@ -34,16 +34,45 @@

/**
* Checks if a URL is allowed based on HTTP_DENY_LIST environment variable
* and built-in SSRF protections (scheme and hostname/IP restrictions).
* @param url - URL to check
* @throws Error if URL hostname resolves to a denied IP
* @throws Error if URL hostname or resolved IP is denied
*/
export async function checkDenyList(url: string): Promise<void> {
const urlObj = new URL(url)
const hostname = urlObj.hostname
const protocol = urlObj.protocol.toLowerCase()

// Enforce allowed protocols
if (protocol !== 'http:' && protocol !== 'https:') {
throw new Error(`Access to this URL is denied by policy: unsupported protocol "${protocol}"`)
}

// Block obvious local/metadata hosts regardless of HTTP_DENY_LIST
const loweredHost = hostname.toLowerCase()
const blockedHostnames = new Set<string>([
'localhost',
'127.0.0.1',
'::1',
'169.254.169.254', // AWS/GCP metadata IP
'metadata.google.internal', // GCP metadata hostname
'169.254.169.250',
'169.254.170.2' // AWS ECS task metadata
])
if (blockedHostnames.has(loweredHost)) {
throw new Error('Access to this host is denied by policy.')
}

const httpDenyListString: string | undefined = process.env.HTTP_DENY_LIST
if (!httpDenyListString) return
if (!httpDenyListString) {
// Even if no explicit deny list is configured, still prevent direct IP access
if (ipaddr.isValid(hostname)) {
throw new Error('Access to this host is denied by policy.')
}
return
}

const httpDenyList = httpDenyListString.split(',').map((ip) => ip.trim())
const urlObj = new URL(url)
const hostname = urlObj.hostname

if (ipaddr.isValid(hostname)) {
isDeniedIP(hostname, httpDenyList)
Copilot is powered by AI and may make mistakes. Always verify output.
Copy link
Author

@christopherholland-workday christopherholland-workday Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is by design/an artifact from a previous commit: https://github.com/FlowiseAI/Flowise/blob/main/packages/components/src/httpSecurity.ts#L178

Will check with the team if there are other actions required to fix this


// If it's a successful response (not a redirect), return it
if (response.status < 300 || response.status >= 400) {
Expand All @@ -196,13 +160,7 @@
}

// Resolve the redirect URL (handle relative URLs)
const redirectUrl = new URL(location, currentUrl).toString()

// Validate the redirect URL against the deny list
await checkDenyList(redirectUrl)

// Update current URL for next iteration
currentUrl = redirectUrl
currentUrl = new URL(location, currentUrl).toString()

// Handle method changes for redirects according to HTTP specs
if (response.status === 301 || response.status === 302 || response.status === 303) {
Expand All @@ -219,3 +177,62 @@

throw new Error('Too many redirects')
}

type ResolvedTarget = {
hostname: string
ip: string
family: 4 | 6
protocol: 'http' | 'https'
}

async function resolveAndValidate(url: string): Promise<ResolvedTarget> {
const denyListString = process.env.HTTP_DENY_LIST
Copy link
Contributor

@HenryHengZJ HenryHengZJ Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By forcing every users to specify HTTP_DENY_LIST, its not user/begineer friendly.

The decision we took is to allow users to have the option to specify if needed, otherwise its not blocking by default. Its not secured by default. We have also documented here

Copy link
Author

@christopherholland-workday christopherholland-workday Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, I will revert this

const denyList = denyListString ? denyListString.split(',').map((s) => s.trim()) : null

const u = new URL(url)
const hostname = u.hostname
const protocol: 'http' | 'https' = u.protocol === 'https:' ? 'https' : 'http'

if (ipaddr.isValid(hostname)) {
if (denyList) {
isDeniedIP(hostname, denyList)
}

return {
hostname,
ip: hostname,
family: hostname.includes(':') ? 6 : 4,
protocol
}
}

const records = await dns.lookup(hostname, { all: true })
if (records.length === 0) {
throw new Error(`DNS resolution failed for ${hostname}`)
}

if (denyList) {
for (const r of records) {
isDeniedIP(r.address, denyList)
}
}

const chosen = records.find((r) => r.family === 4) ?? records[0]

return {
hostname,
ip: chosen.address,
family: chosen.family as 4 | 6,
protocol
}
}

function createPinnedAgent(target: ResolvedTarget): http.Agent | https.Agent {
const Agent = target.protocol === 'https' ? https.Agent : http.Agent

return new Agent({
lookup: (_host, _opts, cb) => {
cb(null, target.ip, target.family)
}
})
}