CVE-2023-35946: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') (CWE-22)
value: Gradle is a build tool with a focus on build automation and support for multi-language development. When Gradle writes a dependency into its dependency cache, it uses the dependency's coordinates to compute a file location. With specially crafted dependency coordinates, Gradle can be made to write files into an unintended location. The file may be written outside the dependency cache or over another file in the dependency cache. This vulnerability could be used to poison the dependency cache or overwrite important files elsewhere on the filesystem where the Gradle process has write permissions. Exploiting this vulnerability requires an attacker to have control over a dependency repository used by the Gradle build or have the ability to modify the build's configuration. It is unlikely that this would go unnoticed. A fix has been released in Gradle 7.6.2 and 8.2 to protect against this vulnerability. Gradle will refuse to cache dependencies that have path traversal elements in their dependency coordinates. It is recommended that users upgrade to a patched version. If you are unable to upgrade to Gradle 7.6.2 or 8.2, `dependency verification` will make this vulnerability more difficult to exploit.
CWE description: The product uses external input to construct a pathname that is intended to identify a file or directory that is located underneath a restricted parent directory, but the product does not properly neutralize special elements within the pathname that can cause the pathname to resolve to a location that is outside of the restricted directory.
CVSS v3 base score: 5.5
Published: 2023-06-30T21:15Z
Repository: https://github.com/gradle/gradle
Language: Other
Files changed: subprojects/core/src/main/java/org/gradle/internal/resource/local/DefaultPathKeyFileStore.java, subprojects/core/src/test/groovy/org/gradle/internal/resource/local/DefaultPathKeyFileStoreTest.groovy, subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/CacheResolveIntegrationTest.groovy
Commit date: 2023-06-20 11:57:05 +0200
Commit message: Fix dependency cache path traversal vulnerability

Gradle leverages the protection added to prevent ZipSlip for any path
computation inside a Gradle cache.

See https://github.com/gradle/gradle/security/advisories/GHSA-2h6c-rv6q-494v

Diff: diff --git a/subprojects/core/src/main/java/org/gradle/internal/resource/local/DefaultPathKeyFileStore.java b/subprojects/core/src/main/java/org/gradle/internal/resource/local/DefaultPathKeyFileStore.java
index 208fe8e1f742..8f3f04e629ee 100644
--- a/subprojects/core/src/main/java/org/gradle/internal/resource/local/DefaultPathKeyFileStore.java
+++ b/subprojects/core/src/main/java/org/gradle/internal/resource/local/DefaultPathKeyFileStore.java
@@ -25,15 +25,19 @@
 import org.gradle.api.internal.file.collections.SingleIncludePatternFileTree;
 import org.gradle.internal.UncheckedException;
 import org.gradle.internal.hash.ChecksumService;
 import org.gradle.util.internal.GFileUtils;
 import org.gradle.util.internal.RelativePathUtil;
+import org.gradle.util.internal.ZipSlip;
 
 import java.io.File;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.Set;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
 
 import static org.gradle.internal.FileUtils.hasExtension;
 
 /**
  * File store that accepts the target path as the key for the entry.
@@ -70,17 +74,23 @@ public DefaultPathKeyFileStore(ChecksumService checksumService, File baseDir) {
     protected File getBaseDir() {
         return baseDir;
     }
 
     private File getFile(String... path) {
-        File result = baseDir;
-        for (String p : path) {
-            result = new File(result, p);
+        String composedPath;
+        if (path.length == 1) {
+            composedPath = path[0];
+        } else {
+            // We need to ignore empty Strings as this is what "new File(parent, path)" was doing for "path" empty.
+            composedPath = Arrays.stream(path)
+                .filter(((Predicate<String>) String::isEmpty).negate())
+                .collect(Collectors.joining(File.separator));
         }
-        return result;
+        return new File(baseDir, ZipSlip.safeZipEntryName(trimLeadingSlash(composedPath)));
     }
 
+
     private File getFileWhileCleaningInProgress(String... path) {
         File file = getFile(path);
         File markerFile = getInProgressMarkerFile(file);
         if (markerFile.exists()) {
             deleteFileQuietly(file);
@@ -210,6 +220,12 @@ public LocallyAvailableResource get(String... path) {
 
     @SuppressWarnings("ResultOfMethodCallIgnored")
     private static void deleteFileQuietly(File file) {
         file.delete();
     }
-}
+
+    private static String trimLeadingSlash(String composedPath) {
+        if (!composedPath.isEmpty() && composedPath.charAt(0) == '/') {
+            return composedPath.substring(1);
+        }
+        return composedPath;
+    }}
diff --git a/subprojects/core/src/test/groovy/org/gradle/internal/resource/local/DefaultPathKeyFileStoreTest.groovy b/subprojects/core/src/test/groovy/org/gradle/internal/resource/local/DefaultPathKeyFileStoreTest.groovy
index b8c78f9ab2d9..b4be34d9a354 100644
--- a/subprojects/core/src/test/groovy/org/gradle/internal/resource/local/DefaultPathKeyFileStoreTest.groovy
+++ b/subprojects/core/src/test/groovy/org/gradle/internal/resource/local/DefaultPathKeyFileStoreTest.groovy
@@ -40,11 +40,12 @@ class DefaultPathKeyFileStoreTest extends Specification {
     def "can move file to filestore"() {
         def a = createFile("abc")
         def b = createFile("def")
 
         when:
-        store.move("a", a)
+        // leading slash does not mean absolute path
+        store.move("/a", a)
         store.move("b", b)
 
         then:
         def storedA = store.get("a")
         storedA.file.text == "abc"
diff --git a/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/CacheResolveIntegrationTest.groovy b/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/CacheResolveIntegrationTest.groovy
index 8aa9cc7716ac..46125d078012 100644
--- a/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/CacheResolveIntegrationTest.groovy
+++ b/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/CacheResolveIntegrationTest.groovy
@@ -18,10 +18,12 @@ package org.gradle.integtests.resolve
 import org.gradle.api.internal.artifacts.ivyservice.CacheLayout
 import org.gradle.cache.internal.scopes.DefaultCacheScopeMapping
 import org.gradle.integtests.fixtures.AbstractHttpDependencyResolutionTest
 import org.gradle.integtests.fixtures.ToBeFixedForConfigurationCache
 import org.gradle.integtests.fixtures.cache.CachingIntegrationFixture
+import org.gradle.internal.hash.Hashing
+import org.gradle.test.fixtures.file.TestFile
 
 import java.nio.file.Files
 
 class CacheResolveIntegrationTest extends AbstractHttpDependencyResolutionTest implements CachingIntegrationFixture {
 
@@ -151,10 +153,159 @@ task listJars {
 
         then:
         succeeds('listJars')
     }
 
+    def 'cannot write cache entries outside of GAV'() {
+        given:
+        def fakeDep = temporaryFolder.testDirectory.file('fake-repo/pwned.txt')
+        fakeDep << """
+Hello world!
+"""
+        def hash = Hashing.sha1().hashFile(fakeDep).toString()
+        def hashOfBootJar = '1234' // for demo purpose
+        def invalidPath = "org.spring/core/1.0/$hash/artifact-1.0./../../../../boot/2.0/$hashOfBootJar/pwned.txt"
+        def invalidLocation = executer.gradleUserHomeDir.file(cachePath + invalidPath).canonicalFile
+
+        server.allowGetOrHead("/repo/org/boot/2.0/$hashOfBootJar/pwned.txt", fakeDep)
+
+        and:
+        withValidJavaSource()
+        buildWithJavaLibraryAndMavenRepoArtifactOnly()
+
+        and:
+        buildFile << """
+dependencies { implementation 'org.spring:core:1.0@/../../../../boot/2.0/$hashOfBootJar/pwned.txt' }
+"""
+
+        when:
+        fails('compileJava')
+
+        then:
+        failureCauseContains('is not a safe zip entry name')
+        // If the build did not fail, Gradle would effectively write a file inside org.spring/boot/2.0 instead of inside org.spring/core/1.0
+        // If we have the real hash of a JAR in those other coordinates, Gradle could overwrite and replace the real JAR with a malicious one
+        !invalidLocation.exists()
+    }
+
+    def 'cannot write cache entries outside of dependency cache'() {
+        given:
+        def fakeDep = temporaryFolder.testDirectory.file('fake-repo/pwned.txt')
+        fakeDep << """
+Hello world!
+"""
+        // Code block used to verify what happens if the build succeeds
+        def hash = Hashing.sha1().hashFile(fakeDep).toString()
+        def invalidPath = "org.spring/../../../../../core/1.0/$hash/artifact-1.0./../../../../.ssh/pwned.txt"
+        def invalidLocation = executer.gradleUserHomeDir.file(cachePath + invalidPath).canonicalFile
+
+        server.allowGetOrHead('/repo/org/.ssh/pwned.txt', fakeDep)
+
+        and:
+        withValidJavaSource()
+        buildWithJavaLibraryAndMavenRepoArtifactOnly()
+
+        and:
+        buildFile << """
+dependencies { implementation 'org.spring/../../../../../:core:1.0@/../../../../.ssh/pwned.txt' }
+"""
+
+        when:
+        fails('compileJava')
+
+        then:
+        failureCauseContains('is not a safe zip entry name')
+        // If the build did not fail, Gradle would effectively write a file inside a folder that is a sibling to the Gradle User Home
+        // If this was ~/.gradle, Gradle would have written in ~/.ssh
+        !invalidLocation.exists()
+    }
+
+    def 'cannot write cache entries anywhere on disk using metadata'() {
+        given:
+        // Our crafty coordinates
+        def pwnedDep = mavenRepo.module('org.spring/../../../../../', 'core')
+        // Our abused coordinates that will see a POM request
+        def abusedCoordinates = mavenHttpRepo.module('org.spring', 'core', '1.0').publish()
+        // Defeat the Gradle validation that will verify metadata content match requested coordinates
+        abusedCoordinates.pom.file.replace('<groupId>org.spring</groupId>', '<groupId>org.spring/../../../../../</groupId>')
+        // Our test dependency that now has a crafty dependency itself
+        def testDep = mavenHttpRepo.module('org.test', 'test').dependsOn(pwnedDep, type: '/../../../../.ssh/pwned.txt').publish()
+
+        def fakeDep = temporaryFolder.testDirectory.file('fake-repo/pwned.txt')
+        fakeDep << """
+Hello world!
+"""
+        def hash = Hashing.sha1().hashFile(fakeDep).toString()
+        def invalidPath = "org.spring/../../../../../core/1.0/$hash/artifact-1.0./../../../../.ssh/pwned.txt"
+        def invalidLocation = executer.gradleUserHomeDir.file(cachePath + invalidPath).canonicalFile
+
+        testDep.allowAll()
+        abusedCoordinates.allowAll()
+        server.allowGetOrHead('/repo/org/.ssh/pwned.txt', fakeDep)
+
+        and:
+        withValidJavaSource()
+        buildWithJavaLibraryAndMavenRepo()
+
+        and:
+        buildFile << """
+dependencies { implementation 'org.test:test:1.0' }
+"""
+
+        when:
+        fails('compileJava')
+
+        then:
+        failureCauseContains('is not a safe zip entry name')
+        // If the build did not fail, Gradle would effectively write a file inside a folder that is a sibling to the Gradle User Home
+        // If this was ~/.gradle, Gradle would have written in ~/.ssh
+        !invalidLocation.exists()
+    }
+
+    private String getCachePath() {
+        "caches/${CacheLayout.ROOT.key}/${CacheLayout.FILE_STORE.key}/"
+    }
+
+    private void buildWithJavaLibraryAndMavenRepoArtifactOnly() {
+        buildFile << """
+plugins {
+    id('java-library')
+}
+
+repositories {
+    maven {
+        url "${mavenHttpRepo.uri}"
+        metadataSources {
+            artifact()
+        }
+    }
+}
+"""
+    }
+
+    private void buildWithJavaLibraryAndMavenRepo() {
+        buildFile << """
+plugins {
+    id('java-library')
+}
+
+repositories {
+    maven {
+        url "${mavenHttpRepo.uri}"
+    }
+}
+"""
+    }
+
+    private TestFile withValidJavaSource() {
+        temporaryFolder.testDirectory.file('src/main/java/org/test/Base.java') << """
+package org.test;
+
+public class Base {}
+"""
+    }
+
     def relocateCachesAndChangeGradleHome() {
         def otherHome = executer.gradleUserHomeDir.parentFile.createDir('other-home')
         def otherCacheDir = otherHome.toPath().resolve(DefaultCacheScopeMapping.GLOBAL_CACHE_DIR_NAME)
         Files.createDirectory(otherCacheDir)
         Files.move(getMetadataCacheDir().toPath(), otherCacheDir.resolve(CacheLayout.ROOT.key))

Vulnerable code:         File result = baseDir;
        for (String p : path) {
            result = new File(result, p);
        return result;
}
        store.move("a", a)
Fixed code: import org.gradle.util.internal.ZipSlip;
import java.util.Arrays;
import java.util.function.Predicate;
import java.util.stream.Collectors;
        String composedPath;
        if (path.length == 1) {
            composedPath = path[0];
        } else {
            // We need to ignore empty Strings as this is what "new File(parent, path)" was doing for "path" empty.
            composedPath = Arrays.stream(path)
                .filter(((Predicate<String>) String::isEmpty).negate())
                .collect(Collectors.joining(File.separator));
        return new File(baseDir, ZipSlip.safeZipEntryName(trimLeadingSlash(composedPath)));


    private static String trimLeadingSlash(String composedPath) {
        if (!composedPath.isEmpty() && composedPath.charAt(0) == '/') {
            return composedPath.substring(1);
        }
        return composedPath;
    }}
        // leading slash does not mean absolute path
        store.move("/a", a)
import org.gradle.internal.hash.Hashing
import org.gradle.test.fixtures.file.TestFile
    def 'cannot write cache entries outside of GAV'() {
        given:
        def fakeDep = temporaryFolder.testDirectory.file('fake-repo/pwned.txt')
        fakeDep << """
Hello world!
"""
        def hash = Hashing.sha1().hashFile(fakeDep).toString()
        def hashOfBootJar = '1234' // for demo purpose
        def invalidPath = "org.spring/core/1.0/$hash/artifact-1.0./../../../../boot/2.0/$hashOfBootJar/pwned.txt"
        def invalidLocation = executer.gradleUserHomeDir.file(cachePath + invalidPath).canonicalFile

        server.allowGetOrHead("/repo/org/boot/2.0/$hashOfBootJar/pwned.txt", fakeDep)

        and:
        withValidJavaSource()
        buildWithJavaLibraryAndMavenRepoArtifactOnly()

        and:
        buildFile << """
dependencies { implementation 'org.spring:core:1.0@/../../../../boot/2.0/$hashOfBootJar/pwned.txt' }
"""

        when:
        fails('compileJava')

        then:
        failureCauseContains('is not a safe zip entry name')
        // If the build did not fail, Gradle would effectively write a file inside org.spring/boot/2.0 instead of inside org.spring/core/1.0
        // If we have the real hash of a JAR in those other coordinates, Gradle could overwrite and replace the real JAR with a malicious one
        !invalidLocation.exists()
    }

    def 'cannot write cache entries outside of dependency cache'() {
        given:
        def fakeDep = temporaryFolder.testDirectory.file('fake-repo/pwned.txt')
        fakeDep << """
Hello world!
"""
        // Code block used to verify what happens if the build succeeds
        def hash = Hashing.sha1().hashFile(fakeDep).toString()
        def invalidPath = "org.spring/../../../../../core/1.0/$hash/artifact-1.0./../../../../.ssh/pwned.txt"
        def invalidLocation = executer.gradleUserHomeDir.file(cachePath + invalidPath).canonicalFile

        server.allowGetOrHead('/repo/org/.ssh/pwned.txt', fakeDep)

        and:
        withValidJavaSource()
        buildWithJavaLibraryAndMavenRepoArtifactOnly()

        and:
        buildFile << """
dependencies { implementation 'org.spring/../../../../../:core:1.0@/../../../../.ssh/pwned.txt' }
"""

        when:
        fails('compileJava')

        then:
        failureCauseContains('is not a safe zip entry name')
        // If the build did not fail, Gradle would effectively write a file inside a folder that is a sibling to the Gradle User Home
        // If this was ~/.gradle, Gradle would have written in ~/.ssh
        !invalidLocation.exists()
    }

    def 'cannot write cache entries anywhere on disk using metadata'() {
        given:
        // Our crafty coordinates
        def pwnedDep = mavenRepo.module('org.spring/../../../../../', 'core')
        // Our abused coordinates that will see a POM request
        def abusedCoordinates = mavenHttpRepo.module('org.spring', 'core', '1.0').publish()
        // Defeat the Gradle validation that will verify metadata content match requested coordinates
        abusedCoordinates.pom.file.replace('<groupId>org.spring</groupId>', '<groupId>org.spring/../../../../../</groupId>')
        // Our test dependency that now has a crafty dependency itself
        def testDep = mavenHttpRepo.module('org.test', 'test').dependsOn(pwnedDep, type: '/../../../../.ssh/pwned.txt').publish()

        def fakeDep = temporaryFolder.testDirectory.file('fake-repo/pwned.txt')
        fakeDep << """
Hello world!
"""
        def hash = Hashing.sha1().hashFile(fakeDep).toString()
        def invalidPath = "org.spring/../../../../../core/1.0/$hash/artifact-1.0./../../../../.ssh/pwned.txt"
        def invalidLocation = executer.gradleUserHomeDir.file(cachePath + invalidPath).canonicalFile

        testDep.allowAll()
        abusedCoordinates.allowAll()
        server.allowGetOrHead('/repo/org/.ssh/pwned.txt', fakeDep)

        and:
        withValidJavaSource()
        buildWithJavaLibraryAndMavenRepo()

        and:
        buildFile << """
dependencies { implementation 'org.test:test:1.0' }
"""

        when:
        fails('compileJava')

        then:
        failureCauseContains('is not a safe zip entry name')
        // If the build did not fail, Gradle would effectively write a file inside a folder that is a sibling to the Gradle User Home
        // If this was ~/.gradle, Gradle would have written in ~/.ssh
        !invalidLocation.exists()
    }

    private String getCachePath() {
        "caches/${CacheLayout.ROOT.key}/${CacheLayout.FILE_STORE.key}/"
    }

    private void buildWithJavaLibraryAndMavenRepoArtifactOnly() {
        buildFile << """
plugins {
    id('java-library')
}

repositories {
    maven {
        url "${mavenHttpRepo.uri}"
        metadataSources {
            artifact()
        }
    }
}
"""
    }

    private void buildWithJavaLibraryAndMavenRepo() {
        buildFile << """
plugins {
    id('java-library')
}

repositories {
    maven {
        url "${mavenHttpRepo.uri}"
    }
}
"""
    }

    private TestFile withValidJavaSource() {
        temporaryFolder.testDirectory.file('src/main/java/org/test/Base.java') << """
package org.test;

public class Base {}
"""
    }