fix(security): enforce SSRF ipBlackList on IPv6 addresses#5967
Conversation
The SSRF address check skipped every IPv6 address and then fell through to "allow", so a hostname resolving to an internal IPv6 address (or an IPv4-mapped IPv6 such as `::ffff:127.0.0.1`) bypassed `ssrf.ipBlackList`. `@eggjs/ip`'s `cidrSubnet().contains()` is also IPv4-only, so it cannot evaluate IPv6 ranges at all. Match IPv6 addresses against the configured rules with a buffer-based prefix comparison, and also check IPv4-mapped IPv6 addresses against IPv4 rules.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces IPv6 support for SSRF protection in the security plugin. It adds handling for IPv4-mapped IPv6 addresses to prevent SSRF bypasses and refactors IP and CIDR subnet checks to use normalized buffer comparisons. The review feedback highlights critical security and robustness improvements: replacing the regex-based IPv4-mapped IPv6 detection with a robust buffer-based check to prevent bypasses via alternative textual representations, validating CIDR prefix lengths to avoid incorrect matches, removing the now-redundant regex, and expanding test coverage to include alternative IPv6 address formats.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const candidates = [address]; | ||
| const mapped = IPV4_MAPPED_RE.exec(address); | ||
| if (mapped) { | ||
| candidates.push(mapped[1]); | ||
| } |
There was a problem hiding this comment.
Using a regular expression to detect and extract IPv4-mapped IPv6 addresses is prone to bypasses because IPv6 addresses can be represented in multiple valid textual formats (e.g., 0:0:0:0:0:ffff:127.0.0.1 or ::ffff:7f00:1). An attacker could potentially use an alternative representation to bypass the regex check while still targeting the internal IPv4 address.
Instead, we can perform a robust buffer-based check. Since IP.toBuffer normalizes all valid representations of an IP address, we can check if the resulting 16-byte buffer represents an IPv4-mapped IPv6 address (where the first 10 bytes are 0 and the next 2 bytes are 0xff), and then extract the IPv4 address directly from the last 4 bytes.
const candidates = [address];
const addressBuffer = toBufferOrNull(address);
if (addressBuffer && addressBuffer.length === 16) {
let isMapped = addressBuffer[10] === 0xff && addressBuffer[11] === 0xff;
if (isMapped) {
for (let i = 0; i < 10; i++) {
if (addressBuffer[i] !== 0) {
isMapped = false;
break;
}
}
}
if (isMapped) {
candidates.push(`${addressBuffer[12]}.${addressBuffer[13]}.${addressBuffer[14]}.${addressBuffer[15]}`);
}
}| const [base, prefix] = rule.split('/'); | ||
| const prefixLength = Number.parseInt(prefix, 10); | ||
| if (!base || Number.isNaN(prefixLength)) { | ||
| throw new Error(`invalid CIDR subnet: ${rule}`); | ||
| } | ||
| const baseBuffer = IP.toBuffer(base); |
There was a problem hiding this comment.
The parsed prefixLength is not validated to ensure it is within valid bounds. If a user configures an invalid CIDR with a negative prefix length (e.g., 1.2.3.4/-5), prefixLength will be negative, causing isInSubnet to skip the comparison loop and incorrectly return true (matching any IP address). Similarly, a prefix length greater than the address's bit length (32 for IPv4, 128 for IPv6) is invalid.
We should validate that prefixLength is non-negative and does not exceed the maximum bit length of the IP family.
const [base, prefix] = rule.split('/');
const prefixLength = Number.parseInt(prefix, 10);
if (!base || Number.isNaN(prefixLength)) {
throw new Error(`invalid CIDR subnet: ${rule}`);
}
const baseBuffer = IP.toBuffer(base);
if (prefixLength < 0 || prefixLength > baseBuffer.length * 8) {
throw new Error(`invalid CIDR subnet: ${rule}`);
}| // IPv4-mapped IPv6 address, e.g. `::ffff:127.0.0.1` | ||
| const IPV4_MAPPED_RE = /^::ffff:(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})$/i; |
There was a problem hiding this comment.
Since we are switching to a robust buffer-based check to detect IPv4-mapped IPv6 addresses, the IPV4_MAPPED_RE regular expression is no longer needed and can be safely removed.
| it('should block IPv4-mapped IPv6 address against IPv4 blacklist rules', () => { | ||
| const checkAddress = buildCheckAddress(['127.0.0.1', '10.0.0.0/8']); | ||
| expect(checkAddress([{ address: '::ffff:127.0.0.1', family: 6 }], 6, 'evil.example.com')).toBe(false); | ||
| expect(checkAddress([{ address: '::ffff:10.1.2.3', family: 6 }], 6, 'evil.example.com')).toBe(false); | ||
| }); |
There was a problem hiding this comment.
To ensure the robustness of the buffer-based IPv4-mapped IPv6 detection, we should add test cases covering alternative valid representations of IPv4-mapped IPv6 addresses (such as using full zero expansion or hex representation for the IPv4 part).
it('should block IPv4-mapped IPv6 address against IPv4 blacklist rules', () => {
const checkAddress = buildCheckAddress(['127.0.0.1', '10.0.0.0/8']);
expect(checkAddress([{ address: '::ffff:127.0.0.1', family: 6 }], 6, 'evil.example.com')).toBe(false);
expect(checkAddress([{ address: '::ffff:10.1.2.3', family: 6 }], 6, 'evil.example.com')).toBe(false);
expect(checkAddress([{ address: '0:0:0:0:0:ffff:127.0.0.1', family: 6 }], 6, 'evil.example.com')).toBe(false);
expect(checkAddress([{ address: '::ffff:7f00:1', family: 6 }], 6, 'evil.example.com')).toBe(false);
});
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## next #5967 +/- ##
==========================================
+ Coverage 85.32% 85.34% +0.02%
==========================================
Files 670 670
Lines 19553 19582 +29
Branches 3864 3870 +6
==========================================
+ Hits 16683 16712 +29
- Misses 2479 2480 +1
+ Partials 391 390 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
1 participant
Problem
The SSRF address check (
config.security.ssrf-> generatedcheckAddress) skipped every IPv6 address and then fell through to "allow":So a hostname that resolves to an internal IPv6 address (or an IPv4-mapped IPv6 such as
::ffff:127.0.0.1, reachable via an attacker-controlled DNS record) bypassedssrf.ipBlackListentirely. On top of that,@eggjs/ip'scidrSubnet().contains()is IPv4-only (it relies on 32-bittoLong), so it cannot evaluate IPv6 CIDR ranges even if the address were checked.Fix
getContainswith a buffer-based prefix comparison that is correct for both IPv4 and IPv6 (and rejects cross-family comparisons).::ffff:127.0.0.1is caught by a127.0.0.1entry.Test
Added unit tests on the generated
checkAddresscovering: IPv6 addresses in the blacklist, IPv4-mapped IPv6 against IPv4 rules, andipExceptionListfor IPv6. All fail before the fix (addresses were allowed) and pass after. Existing IPv4 behavior is unchanged.plugins/securitysuite: 201 passed, 4 skipped.