Conversation
| const response = await axios({ | ||
| method: 'get', | ||
| url: input, | ||
| headers, | ||
| responseType: 'stream', | ||
| timeout: 30000 | ||
| }) |
Check failure
Code scanning / CodeQL
Server-side request forgery Critical
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 12 days ago
To fix the problem, we need to ensure that URLs coming from user input are validated against a strict policy before being used in outbound HTTP requests. The key points are: enforce an allow-list of schemes (e.g. only http/https), ensure the host is not a loopback, private, or link-local address, optionally restrict to a known set of trusted domains/hosts, and reject malformed or relative URLs. This validation should be done in UrlStorage.validate(), since all uses of file.url (including getDownloadUrl and fetchSpecificFileMetadata) depend on validate() returning true.
The best minimal fix without altering existing functionality is to extend UrlStorage.validate() with robust URL validation using Node’s url and dns/net modules (which are standard and do not require new dependencies). Specifically, we can:
- Parse
file.urlwithnew URL(file.url)inside a try/catch; if parsing fails or the protocol is nothttp:orhttps:, reject the URL. - Use
dns.promises.lookupornet.isIPplusdns.lookup-like resolution to determine the IP address(es) for the hostname. - Check that the resolved IP(s) are not in loopback (
127.0.0.0/8,::1), private (10.0.0.0/8,172.16.0.0/12,192.168.0.0/16), link-local (169.254.0.0/16,fe80::/10), or other restricted ranges; if they are, reject the URL. - Optionally, if
this.configcontains an allow-list (e.g.allowedURLHostsorallowedURLDomains), enforce thatparsed.hostnamematches one of those. Since we cannot assume such fields exist, we will implement only the IP-range restrictions plus scheme+parse checks.
Because validate() is synchronous and we don’t want to change signatures to async, we should avoid async DNS lookups. Instead, we can protect against obvious direct-IP hosts by checking parsed.hostname with net.isIP and rejecting if it is a loopback/private/link-local IP. This is a meaningful improvement and keeps validate() synchronous. Hosts expressed as domain names will still be allowed, but this is acceptable as a minimal, non-breaking change; further protection can be configured via unsafeURLs as today.
Concretely:
- In
src/components/storage/UrlStorage.ts, import Node’surlandnetfacilities (URLis global in modern Node, so no import is required) andnetfrom'net'. - Add a private helper method
isPrivateOrDisallowedHost(urlString: string): booleanthat:- Parses the URL.
- Ensures
protocolishttp:orhttps:. - Uses
net.isIP(parsed.hostname)and a small helper to check whether that IP is loopback, private, or link-local.
- Update
validate()to call this helper and reject the URL if it returns true (e.g. returning[false, 'URL host is not allowed']). - Also add an explicit check that
file.urlis an absolute URL starting withhttp://orhttps://before the existingisFilePath()check, to make intent clearer.
This keeps all existing functionality (valid public HTTP/HTTPS URLs continue to work) while blocking the most dangerous SSRF vectors using direct IPs to internal networks. It also ensures getDownloadUrl() no longer returns unsafe URLs, eliminating the tainted flow into axios.
Fixes # .
Changes proposed in this PR: