Initial commit of safe url resolver#1910
Conversation
| return false; | ||
| } | ||
|
|
||
| String host = IDN.toASCII(parsed.getHost(), IDN.ALLOW_UNASSIGNED); |
There was a problem hiding this comment.
Does IDN.toASCII handle the wildcards ok? The Javadoc says IllegalArgumentException - if the input string doesn't conform to RFC 3490 specification. I really can't think of a realistic attack vector for this so if it's problematic feel free to remove it.
There was a problem hiding this comment.
so IDN.toASCII here is only parsing the end urls submitted by the user and seems to work fine with "incorrect" urls like "https://ex*.com/page" as it is using IDN.ALLOW_UNASSIGNED.
This is required to be able to parse emojis as punycode and any other non-standard characters
|
Great stuff. LGTM at least. Will there be another PR to enable swagger-parser to be configured to use it? |
@bcoughlan Yes indeed, separate tickets created for parser and codegen implementations that will pull this lib in |
| <dependency> | ||
| <groupId>org.testng</groupId> | ||
| <artifactId>testng</artifactId> | ||
| <version>7.7.0</version> |
There was a problem hiding this comment.
I'd go for ${testng-version}
| @@ -0,0 +1,97 @@ | |||
| package io.swagger.parser.v3.urlresolver; | |||
There was a problem hiding this comment.
To be consistent with the project/artifacts package naming this should be io.swagger.v3.parser...
Applicable to all files
There was a problem hiding this comment.
Updated across the packages now
| private final UrlPatternMatcher allowlistMatcher; | ||
| private final UrlPatternMatcher denylistMatcher; |
There was a problem hiding this comment.
maybe as protected to allow for extensibility?
There was a problem hiding this comment.
Absolutely, will update presently
| return new ResolvedUrl(urlWithIp, hostname); | ||
| } | ||
|
|
||
| private boolean isRestrictedIpRange(InetAddress ip) { |
There was a problem hiding this comment.
maybe as protected to allow for extensibility?
This PR adds a new module to the swagger parser repo:
swagger-parser-safe-url-resolverWhat is it?
The main class of the package is the
PermittedUrlsCheckerwhich has one method:verify(String url).This method takes in a string URL and performs the following steps:
ResolvedUrlobject which contains4.1. A
String urlwhere the original URL has the hostname replaced with the IP address4.2. A
String hostHeaderwhich contains the hostname from the original URL to be added as a host headerThis behavior can also be customized with the
allowlistanddenylistin the constructor, whereby:allowlistwill allow the URL to pass even if it resolves to a private/restricted IP addressdenylistwill throw an exception even when the URL resolves to a public IP addressWhy is it needed?
This lib can be used in services that deal with user-submitted URLs that get fetched (like in swagger-parser resolving external URL $refs) to protect against Server-Side Request Forgery and DNS rebinding attacks
Example usage