Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sandboxed URL creation to prevent SSRF attacks #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pixeebot[bot]
Copy link

@pixeebot pixeebot bot commented Mar 22, 2024

This change sandboxes the creation of java.net.URL objects so they will be more resistant to Server-Side Request Forgery (SSRF) attacks.

Most of the time when you create a URL, you're intending to reference an HTTP endpoint, like an internal microservice. However, URLs can point to local file system files, a Gopher stream in your local network, a JAR file on a remote Internet site, and all kinds of other unexpected and undesirable stuff. When the URL values are influenced by attackers, they can trick your application into fetching internal resources, running malicious code, or otherwise harming the system. Consider the following code:

String url = userInput.getServiceAddress();
return IOUtils.toString(new URL(url).openConnection());

In this case, an attacker could supply a value like jar:file:/path/to/appserver/lib.jar and attempt to read the contents of your application's code.

Our changes introduce sandboxing around URL creation that force the developers to specify some boundaries on the types of URLs they expect to create:

+ import io.github.pixee.security.Urls;
+ import io.github.pixee.security.HostValidator;
  ...
  String url = userInput.getServiceAddress();
- URL u = new URL(url);
+ URL u = Urls.create(url, Urls.HTTP_PROTOCOLS, HostValidator.DENY_COMMON_INFRASTRUCTURE_TARGETS);
  InputStream is = u.openConnection();

This change alone reduces attack surface significantly, but can be enhanced to create even more security by specifying some controls around the hosts we expect to connect with:

+ import io.github.pixee.security.Urls;
+ import io.github.pixee.security.HostValidator;
  ...
  HostValidator allowsOnlyGoodDotCom = HostValidator.fromAllowedHostPattern(Pattern.compile("good\\.com"));
  URL u = Urls.create(url, Urls.HTTP_PROTOCOLS, allowsOnlyGoodDotCom);

Note: Beware temptation to write some validation on your own. Parsing URLs is difficult and differences between parsers in validation and execution will certainly lead to exploits as attackers have repeatedly proven.

❌ The following packages couldn't be installed automatically, probably because the dependency manager is unsupported. Please install them manually:

Gradle
dependencies {
  implementation("io.github.pixee:java-security-toolkit:1.1.3")
}
Maven
<dependencies>
  <dependency>
    <groupId>io.github.pixee</groupId>
    <artifactId>java-security-toolkit</artifactId>
    <version>1.1.3</version>
  </dependency>
<dependencies>
More reading

Powered by: pixeebot (codemod ID: pixee:java/sandbox-url-creation)

Copy link
Author

pixeebot bot commented Mar 26, 2024

I'm confident in this change, but I'm not a maintainer of this project. Do you see any reason not to merge it?

If this change was not helpful, or you have suggestions for improvements, please let me know!

Copy link
Author

pixeebot bot commented Apr 2, 2024

Besides the SSRF risks described above, a control like the one I suggested will also help protect against URL parsing bugs where attackers can present a URL that looks like one thing, but ends up being acted on as something else. A leading hacker in this space, Orange Tsai, has a nice visual of showing how humans and parsers will see the same URL differently in his legendary BlackHat talk:

image

This is why I wanted to introduce an API that forces you to provide the expectations of the URL at parsing time. There's really no downside, and this code will leave practically no opportunity of ending up in a confused and possibly vulnerable state. The code will be more clear and more safe.

If there are other concerns about this change, I'd love to hear about them!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

0 participants