Improve IPv6 address validation logic#79
Conversation
Enhance IPv6 address validation by adding checks for IPv4-mapped and IPv4-compatible addresses, along with additional manual blocks for specific IPv6 ranges.
Review Summary by QodoEnhance IPv6 and URL hostname validation security
WalkthroughsDescription• Enhanced IPv6 address validation to detect IPv4-mapped and IPv4-compatible addresses • Added manual blocks for specific IPv6 ranges (64:ff9b:1::/48, 5f00::/8, 3fff::/20, fec0::/10) • Fixed hostname parsing in URL validation to strip IPv6 bracket notation • Improved internal IP detection logic for security vulnerability mitigation Diagramflowchart LR
A["IPv6 Validation"] --> B["Check IPv4-mapped/compatible"]
B --> C["Recursively validate as IPv4"]
A --> D["Check extra bad ranges"]
D --> E["Match against CIDR blocks"]
F["URL Hostname Parsing"] --> G["Strip IPv6 brackets"]
G --> H["Validate cleaned hostname"]
File Changes1. dist/helpers.js
|
Code Review by Qodo
1. Dist/source logic diverges
|
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
Up to standards ✅🟢 Issues
|
| if (range !== "unicast") { | ||
| // Check for IPv4-mapped or IPv4-compatible addresses | ||
| if (range === "ipv4Mapped" || range === "rfc6145") { | ||
| try { | ||
| // @ts-ignore | ||
| const ipv4 = parsed.toIPv4Address(); | ||
| return is_ip_internal(ipv4.toString()); | ||
| } | ||
| catch { | ||
| return true; | ||
| } | ||
| } | ||
| return true; | ||
| } | ||
| // Additional manual blocks for IPv6 | ||
| const extraBadRanges = [ | ||
| ipaddr.parseCIDR("64:ff9b:1::/48"), | ||
| ipaddr.parseCIDR("5f00::/8"), | ||
| ipaddr.parseCIDR("3fff::/20"), | ||
| ipaddr.parseCIDR("fec0::/10"), | ||
| ]; | ||
| for (const [range, bits] of extraBadRanges) { | ||
| // @ts-ignore | ||
| if (parsed.match(range, bits)) | ||
| return true; | ||
| } |
There was a problem hiding this comment.
1. Dist/source logic diverges 🐞 Bug ⛨ Security
The IPv6/URL hostname hardening was applied only in dist/helpers.js, while src/helpers.ts still contains the old logic; running tsc (the project build) will overwrite dist and drop the fix. Code paths importing from src/ (e.g., benches) will also continue using the old behavior, undermining the intended GHSA fix.
Agent Prompt
### Issue description
Security-related IPv6/hostname validation changes were made directly in the generated file `dist/helpers.js` only. The authoritative source `src/helpers.ts` still has the previous behavior, so `npm run build` (`tsc`) will overwrite the fix and internal repo imports from `src/` will remain vulnerable.
### Issue Context
- `tsconfig.json` outputs to `dist` and includes only `src`.
- `package.json` build script is `tsc`.
- Benchmarks import from `../src/utils`.
### Fix Focus Areas
- src/helpers.ts[224-243]
- src/helpers.ts[482-520]
- dist/helpers.js[215-252]
- dist/helpers.js[425-460]
### What to do
1. Implement the same IPv6 validation logic (including IPv4-mapped handling + extra blocked ranges) in `src/helpers.ts:is_ip_internal`.
2. Apply the same hostname normalization (`parsed.hostname.replace(...)`) in `src/helpers.ts:is_url_safe` (and debug variant if intended).
3. Re-run `npm run build` to regenerate `dist/` from `src/` and commit the generated output so `dist/` matches `src/`.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Fix GHSA-8p33-q827-ghj5