C0oki3s patch 2#8
Conversation
Refactor ssrf module to include configuration options for blacklist, allowlist, and path behavior. Enhance URL validation and DNS checks to improve security against SSRF attacks.
Updated dns.lookup to return all addresses and adjusted bitwise operations for consistency.
There was a problem hiding this comment.
Pull Request Overview
This PR significantly refactors the SSRF (Server-Side Request Forgery) protection module to improve security, maintainability, and functionality. The refactoring replaces the legacy url module with the WHATWG URL API and introduces comprehensive IP address validation capabilities.
Key changes:
- Modernizes URL parsing from deprecated Node.js
urlmodule to standards-compliant WHATWG URL API - Adds support for allowlists, CIDR-based IP filtering, port restrictions, and configurable IP range blocking
- Implements obfuscated IPv4 detection to prevent attackers from bypassing IP checks using hex/octal/dword notation
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| const content = fs.readFileSync(blacklist, 'utf8') | ||
| config.blacklist = normalizeList(content.replace(/\r\n/g, '\n').split('\n')) | ||
| } catch (error) { | ||
| throw new Error('File does not Exists') |
There was a problem hiding this comment.
Corrected spelling of 'Exists' to 'Exist' in error message.
| const content = fs.readFileSync(wl, 'utf8') | ||
| config.allowlist = normalizeList(content.replace(/\r\n/g, '\n').split('\n')) | ||
| } catch (error) { | ||
| throw new Error('File does not Exists') |
There was a problem hiding this comment.
Corrected spelling of 'Exists' to 'Exist' in error message.
|
|
||
| function parseFlexible (s) { | ||
| if (/^0x[0-9a-f]+$/i.test(s)) return parseInt(s, 16) | ||
| if (/^0[0-7]+$/.test(s)) return parseInt(s, 8) |
There was a problem hiding this comment.
Octal parsing pattern /^0[0-7]+$/ requires at least two characters, incorrectly rejecting valid single octal digit '0'. The pattern should be /^0[0-7]*$/ to match '0' and other octal strings correctly.
| if (/^0[0-7]+$/.test(s)) return parseInt(s, 8) | |
| if (/^0[0-7]*$/.test(s)) return parseInt(s, 8) |
| const addresses = await new Promise((resolve, reject) => { | ||
| dns.lookup(hostname, { all: true, family: 0, hints: dns.ADDRCONFIG | dns.V4MAPPED }, (err, addresses) => { |
There was a problem hiding this comment.
The dns.V4MAPPED hint constant may not be available in all Node.js versions or on all platforms. This could cause a runtime error if the constant is undefined. Consider checking for its existence or using a fallback value.
| const addresses = await new Promise((resolve, reject) => { | |
| dns.lookup(hostname, { all: true, family: 0, hints: dns.ADDRCONFIG | dns.V4MAPPED }, (err, addresses) => { | |
| // Use safe fallback for hints if constants are missing | |
| const hints = ((typeof dns.ADDRCONFIG === 'number' ? dns.ADDRCONFIG : 0) | (typeof dns.V4MAPPED === 'number' ? dns.V4MAPPED : 0)); | |
| const addresses = await new Promise((resolve, reject) => { | |
| dns.lookup(hostname, { all: true, family: 0, hints }, (err, addresses) => { |
No description provided.