Skip to content

Improve SSRF IP validation and add protocol allowlist#18518

Merged
FelixMalfait merged 3 commits intomainfrom
fix/ssrf-ipv4-mapped-ipv6-bypass
Mar 10, 2026
Merged

Improve SSRF IP validation and add protocol allowlist#18518
FelixMalfait merged 3 commits intomainfrom
fix/ssrf-ipv4-mapped-ipv6-bypass

Conversation

@FelixMalfait
Copy link
Copy Markdown
Member

Summary

  • Replace regex-based private IP detection in isPrivateIp with Node.js net.BlockList for CIDR-based range checking, which properly handles all IPv4-mapped IPv6 representations (both dotted-decimal and hex forms)
  • Add missing non-routable IP ranges: carrier-grade NAT (100.64.0.0/10), IANA special purpose, documentation networks, benchmarking, multicast, and reserved ranges
  • Add protocol allowlist (http/https only) as an axios request interceptor in SecureHttpClientService and as a Zod refinement in the HTTP tool schema

Test plan

  • All 100 existing + new tests pass across 4 secure-http-client test suites
  • New tests cover carrier-grade NAT range boundaries (100.64.0.0 – 100.127.255.255)
  • New tests cover documentation, benchmarking, multicast, and reserved ranges
  • New tests cover hex-form IPv4-mapped IPv6 addresses (the form Node.js URL parser actually produces)
  • New tests verify protocol interceptor blocks ftp: and file: schemes
  • New tests verify protocol interceptor is only active when safe mode is enabled

Made with Cursor

Node.js URL parser normalizes IPv4-mapped IPv6 addresses to hex form
(e.g. ::ffff:169.254.169.254 → ::ffff:a9fe:a9fe) before they reach
createConnection. The regex-based isPrivateIp only matched dotted-decimal
form, so the hex form bypassed all SSRF protection.

Replace regex-based IP range matching with net.BlockList for CIDR-based
range checking. Extract embedded IPv4 from both hex and dotted forms of
IPv4-mapped IPv6 before checking. This eliminates the entire class of
string normalization bypass vulnerabilities.

Made-with: Cursor
…ol validation

Add missing non-routable IP ranges to the BlockList: carrier-grade NAT
(100.64.0.0/10), IANA special purpose (192.0.0.0/24), documentation
networks (192.0.2.0/24, 198.51.100.0/24, 203.0.113.0/24), benchmarking
(198.18.0.0/15), multicast (224.0.0.0/4), and reserved (240.0.0.0/4).

Add protocol allowlist (http/https only) as an axios interceptor in
SecureHttpClientService and as a Zod refinement in the HTTP tool schema,
blocking non-HTTP schemes like ftp: and file: as defense-in-depth.

Made-with: Cursor
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 5 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/twenty-server/src/engine/core-modules/secure-http-client/secure-http-client.service.ts">

<violation number="1" location="packages/twenty-server/src/engine/core-modules/secure-http-client/secure-http-client.service.ts:67">
P1: Protocol allowlist can be bypassed when `requestConfig.url` is an empty string because validation is skipped before checking `baseURL`.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Empty string url with ?? skips validation since "" is not nullish,
allowing a non-http baseURL to go unchecked. Using || ensures the
baseURL is checked when url is empty.

Made-with: Cursor
@FelixMalfait
Copy link
Copy Markdown
Member Author

FelixMalfait commented Mar 10, 2026

🚀 Preview Environment Ready!

Your preview environment is available at: http://bore.pub:2695

This environment will automatically shut down after 5 hours.

@FelixMalfait FelixMalfait added this pull request to the merge queue Mar 10, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 10, 2026
@FelixMalfait FelixMalfait merged commit 05c2da2 into main Mar 10, 2026
74 checks passed
@FelixMalfait FelixMalfait deleted the fix/ssrf-ipv4-mapped-ipv6-bypass branch March 10, 2026 12:14
@twenty-eng-sync
Copy link
Copy Markdown

Hey @FelixMalfait! After you've done the QA of your Pull Request, you can mark it as done here. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants