[Java] Proposal: path traversal rule for archive-derived and constructed paths

[FORIMOC (签到型CTFer)]

Hi Sonar team,

I would like to ask for feedback on a possible new Java security rule related to path traversal.

While reviewing several Java cases, I noticed a recurring pattern that seems comparatively under-covered: attacker-controlled path components being propagated into file-system paths or resource lookups.

Representative cases include:

• CVE-2022-4494
• CVE-2022-39367
• CVE-2022-31194
• CVE-2022-29253

The common shapes are:

• archive-entry-derived paths such as ZipEntry.getName() / JarEntry.getName()
• request-derived paths propagated through one path-construction step
• template or resource names later reaching ClassLoader.getResource(…)

A representative archive-entry example is:

while ((zipEntry = zipInputStream.getNextEntry()) != null) {

final File destFile = new File(importSandboxDirectory, zipEntry.getName());

ServiceUtilities.ensureFileCreated(destFile);

final FileOutputStream destOutputStream = new FileOutputStream(destFile);

}

A representative request-derived example is:

String resumableIdentifier = request.getParameter(“resumableIdentifier”);

tempDir = tempDir + File.separator + resumableIdentifier;

File fileDir = new File(tempDir);

fileDir.mkdir();

A representative resource-lookup example is:

String templatePath = suffixPath + templateName;

return classloader.getResource(templatePath);

I prepared a conservative prototype that focuses on:

• ZipEntry.getName() / JarEntry.getName()
• getParameter(…)
• conservative path-like method parameters such as path, file, filename, dir, template, or resource
• simple propagation through assignment and string concatenation
• sinks such as new File(…), mkdir(), mkdirs(), createNewFile(), and ClassLoader.getResource(…)

I also validated the prototype on the representative cases above.

The current prototype uses a temporary rule key only for implementation and testing purposes, and I would be happy to align with the usual SonarJava rule-key / RSPEC process.

Before pushing this proposal further, I would really appreciate your feedback on whether this looks like a rule direction that would fit SonarJava’s expectations and rule strategy.

If this seems relevant, I can continue refining the PR accordingly.

Thanks a lot.

[Malte (Malte Skoruppa)]

Hey @FORIMOC ,

Thank you very much for the suggestion and all the work you put into this!

May I ask whether you are using the SonarQube Server Community Build?

The reason I am asking is because, in SonarQube Server Developer Edition and up, as well as on SonarQube Cloud, we perform an involved taint analysis on user code to discover exactly the kind of injection vulnerabilities that you report here.

I looked into your examples in detail and I can confirm that we do support the first two shapes of the vulnerability you reported, and that we do raise an issue on the exact examples you provided, namely:

• Archive-entry-derived paths: We colloquially call this rule Zip Slip (S6096). On your code sample, an issue is raised successfully:

  

• Request-derived paths propagated through one path-construction step: We typically refer to this one as a Path Injection (S2083). Here, too, we raise successfully on the code example you provided:

  

Our taint analysis is not based on variable names such as path, file, filename, etc. Instead, the engine is based on the notion of sources and sinks:

• Sources are pieces of code that retrieve user-provided data (e.g., via methods such as request.getParameter() or parameter annotations; we support a multitude of different frameworks across several languages).
• Sinks are functions that may allow an attacker to perform sensitive operations when they are called with user-controlled data that is not appropriately sanitized/validated.

We do not currently support the third shape you reported. Let me formalize this a bit. The shape you suggest seems to be:

• Source: Request-driven (as for the second shape), e.g., request.getParameter(templateName)
• Sink: Resource lookup, e.g., classloader.getResource(templatePath)

I’d tend to agree that from a security standpoint, this yields some concerns, e.g.:

• If templateName comes from an untrusted source (like a URL parameter ?template=...), an attacker could pass a payload like ../../META-INF/maven/com.example/app/pom.properties or reach internal configuration files (application.properties, secret keys, internal XML files, compiled .class files, etc.) that happen to be packaged inside the JARs or application root, but were never intended to be rendered or served externally.
• As ClassLoader.getResource(...) is frequently used in web applications to fetch dynamic template views (e.g., Freemarker or Thymeleaf templates), then if an attacker controls the template name, they could potentially force the application to render internal system templates.
• …

I will flag this with our AppSec experts. If they do agree that this is something where we’d want to raise, we will absolutely add support for this type of vulnerability.

We do appreciate that you took the time and worked on a prototype. That said, our taint engine is closed-source and if we add support for the third type of injection you suggested, we will implement it using our own mechanisms inside the existing taint engine. Therefore, you do not need to push further on your own implementation.

That said, we do highly appreciate your feedback and the improvement idea you provided! Thank you very much, this is very valuable.

Happy to answer any follow-up questions.

[Malte (Malte Skoruppa)]

Hey @FORIMOC,

Quick follow-up. I discussed the third shape with our AppSec experts, and they agree we should support it.

Where we stand today:

• Shapes 1 and 2 are already detected: archive-derived paths via S6096, request-derived paths via S2083.
• Shape 3 is not: ClassLoader.getResource(...) and friends aren’t configured as sinks, so nothing is raised.

We’ve created a ticket to close that gap. The plan is to add the resource lookup methods as sinks for both S2083 and S6096, since a resource name derived from an archive entry is exposed the same way.

Thanks again for the well-researched report and the CVE references! Your feedback helps us improve!