This is an automated email from the ASF dual-hosted git repository.
paulk-asert pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/master by this push:
new 0fdd426264 GROOVY-12369: grape uninstall: keep artifact deletion
inside the module cache
0fdd426264 is described below
commit 0fdd426264dcb100eee25c207fa2e5eef6e063da
Author: Paul King <[email protected]>
AuthorDate: Sun Sep 6 14:56:04 2026 +1000
GROOVY-12369: grape uninstall: keep artifact deletion inside the module
cache
uninstallArtifact deletes the jars named by a module's cached ivy
descriptor. The jar path was new File(jardir, name) with name taken from
the descriptor's artifact attributes unchecked, so a name carrying a path
separator or a ".." segment deleted a file outside the module's jars
directory:
<artifact name="../victim" .../>
-> new File(jardir, "../victim-1.0.jar").delete() above jardir
The deletion is now skipped, with a warning, unless the resolved file is
inside the jars directory. The check is on canonical paths, so a ".." or a
symbolic link along the way cannot carry it out, and it covers the version,
classifier and extension that are concatenated into the name as well.
Strictly this is out of the security model: a crafted descriptor is cached
only by resolving from a repository, and resolving from an untrusted
repository already runs its code, so a delete-on-uninstall is weaker than
what such a repository already achieved. It is fixed anyway, as defense in
depth and because a tool should not delete outside its own cache whatever a
descriptor says, matching the containment added for deleteDir and groovydoc.
The regression test builds a module cache with a traversing artifact name,
runs uninstall, and asserts the outside file survives while an ordinary
artifact is still deleted; it was confirmed to delete the outside file
without the guard. It runs under -Djunit.network=true with the other
groovy.grape tests, though it needs no network itself.
---
.../main/groovy/groovy/grape/ivy/GrapeIvy.groovy | 26 +++++++
.../ivy/GrapeIvyUninstallContainmentTest.groovy | 90 ++++++++++++++++++++++
2 files changed, 116 insertions(+)
diff --git
a/subprojects/groovy-grape-ivy/src/main/groovy/groovy/grape/ivy/GrapeIvy.groovy
b/subprojects/groovy-grape-ivy/src/main/groovy/groovy/grape/ivy/GrapeIvy.groovy
index b3a3f5e6b5..ed82125bab 100644
---
a/subprojects/groovy-grape-ivy/src/main/groovy/groovy/grape/ivy/GrapeIvy.groovy
+++
b/subprojects/groovy-grape-ivy/src/main/groovy/groovy/grape/ivy/GrapeIvy.groovy
@@ -58,6 +58,7 @@ import org.w3c.dom.Element
import javax.xml.XMLConstants
import javax.xml.parsers.DocumentBuilderFactory
import javax.xml.parsers.ParserConfigurationException
+import java.nio.file.Path
import java.text.ParseException
import java.util.logging.Level
import java.util.logging.Logger
@@ -735,6 +736,12 @@ class GrapeIvy implements GrapeEngine {
if (classifier) name += "-$classifier"
name += ".${attrs.getNamedItem('ext').getTextContent()}"
def jarfile = new File(jardir, name)
+ if (!isWithin(jardir, jarfile)) {
+ // the name comes from a cached descriptor; a value carrying a
path separator or
+ // a ".." segment would otherwise delete a file outside the
module's jars directory
+ System.err.println("Skipping ${name}: artifact path escapes
the module cache directory")
+ continue
+ }
if (jarfile.exists()) {
System.err.println("Deleting ${jarfile.getName()}")
jarfile.delete()
@@ -742,6 +749,25 @@ class GrapeIvy implements GrapeEngine {
}
}
+ /**
+ * Whether {@code file} resolves to a location inside {@code dir}. Uses
canonical paths, so a
+ * {@code ..} segment or a path separator in the name — or a symbolic link
along the way —
+ * cannot carry the resolved file out of the directory.
+ *
+ * @param dir the directory the file must stay within
+ * @param file the candidate file
+ * @return whether the file is contained in the directory
+ */
+ private static boolean isWithin(File dir, File file) {
+ try {
+ Path root = dir.canonicalFile.toPath()
+ Path target = file.canonicalFile.toPath()
+ target.startsWith(root)
+ } catch (IOException ignored) {
+ false
+ }
+ }
+
private addExcludesIfNeeded(Map args, DefaultModuleDescriptor md) {
args.excludes?.each { Map<String, String> map ->
def excludeRule = new DefaultExcludeRule(
diff --git
a/subprojects/groovy-grape-ivy/src/test/groovy/groovy/grape/ivy/GrapeIvyUninstallContainmentTest.groovy
b/subprojects/groovy-grape-ivy/src/test/groovy/groovy/grape/ivy/GrapeIvyUninstallContainmentTest.groovy
new file mode 100644
index 0000000000..4994e38236
--- /dev/null
+++
b/subprojects/groovy-grape-ivy/src/test/groovy/groovy/grape/ivy/GrapeIvyUninstallContainmentTest.groovy
@@ -0,0 +1,90 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package groovy.grape.ivy
+
+import org.junit.jupiter.api.Test
+import org.junit.jupiter.api.io.TempDir
+
+import java.nio.file.Path
+
+/**
+ * {@code uninstallArtifact} deletes the jars named by a cached ivy
descriptor. An artifact name in
+ * that descriptor must not delete a file outside the module's own jars
directory, whatever a
+ * crafted or corrupt descriptor puts in it.
+ */
+final class GrapeIvyUninstallContainmentTest {
+
+ @TempDir
+ Path grapeRoot
+
+ private static String descriptor(String artifactName) {
+ """<?xml version="1.0"?>
+ <ivy-module version="2.0">
+ <info organisation="mygroup" module="mymodule" revision="1.0"/>
+ <publications>
+ <artifact name="${artifactName}" type="jar" ext="jar"/>
+ </publications>
+ </ivy-module>""".stripIndent()
+ }
+
+ private File setUpModule(String artifactName) {
+ File moduleDir = new File(grapeRoot.toFile(),
'grapes/mygroup/mymodule')
+ File jars = new File(moduleDir, 'jars')
+ jars.mkdirs()
+ new File(moduleDir, 'ivy-1.0.xml').text = descriptor(artifactName)
+ moduleDir
+ }
+
+ @Test
+ void aTraversingArtifactNameDoesNotDeleteOutsideTheJarsDirectory() {
+ String previous = System.getProperty('grape.root')
+ System.setProperty('grape.root', grapeRoot.toFile().absolutePath)
+ try {
+ File moduleDir = setUpModule('../victim')
+ // the file the traversal would reach, a level above the jars
directory
+ File victim = new File(moduleDir, 'victim-1.0.jar')
+ victim.text = 'precious'
+
+ new GrapeIvy().uninstallArtifact('mygroup', 'mymodule', '1.0')
+
+ assert victim.exists() : 'a ".." in the artifact name deleted a
file outside the jars directory'
+ } finally {
+ if (previous == null) System.clearProperty('grape.root')
+ else System.setProperty('grape.root', previous)
+ }
+ }
+
+ @Test
+ void anOrdinaryArtifactIsStillDeleted() {
+ String previous = System.getProperty('grape.root')
+ System.setProperty('grape.root', grapeRoot.toFile().absolutePath)
+ try {
+ File moduleDir = setUpModule('goodlib')
+ File jar = new File(moduleDir, 'jars/goodlib-1.0.jar')
+ jar.text = 'contents'
+
+ new GrapeIvy().uninstallArtifact('mygroup', 'mymodule', '1.0')
+
+ assert !jar.exists() : 'a normal artifact in the jars directory
should be deleted'
+ } finally {
+ if (previous == null) System.clearProperty('grape.root')
+ else System.setProperty('grape.root', previous)
+ }
+ }
+}
