Validate coordinates and repository precedence during artifact resolution

maven-core/src/main/java/org/apache/maven/artifact/repository/metadata/io/DefaultMetadataReader.java

@@ -26,6 +26,10 @@
 import java.util.Objects;
 
 import org.apache.maven.artifact.repository.metadata.Metadata;
+import org.apache.maven.artifact.repository.metadata.Plugin;
+import org.apache.maven.artifact.repository.metadata.Snapshot;
+import org.apache.maven.artifact.repository.metadata.SnapshotVersion;
+import org.apache.maven.artifact.repository.metadata.Versioning;
 import org.apache.maven.artifact.repository.metadata.io.xpp3.MetadataXpp3Reader;
 import org.codehaus.plexus.component.annotations.Component;
 import org.codehaus.plexus.util.ReaderFactory;
@@ -51,7 +55,9 @@ public Metadata read(Reader input, Map<String, ?> options) throws IOException {
         Objects.requireNonNull(input, "input cannot be null");
 
         try (Reader in = input) {
-            return new MetadataXpp3Reader().read(in, isStrict(options));
+            Metadata metadata = new MetadataXpp3Reader().read(in, isStrict(options));
+            validateMetadata(metadata);
+            return metadata;
         } catch (XmlPullParserException e) {
             throw new MetadataParseException(e.getMessage(), e.getLineNumber(), e.getColumnNumber(), e);
         }
@@ -61,7 +67,9 @@ public Metadata read(InputStream input, Map<String, ?> options) throws IOExcepti
         Objects.requireNonNull(input, "input cannot be null");
 
         try (InputStream in = input) {
-            return new MetadataXpp3Reader().read(in, isStrict(options));
+            Metadata metadata = new MetadataXpp3Reader().read(in, isStrict(options));
+            validateMetadata(metadata);
+            return metadata;
         } catch (XmlPullParserException e) {
             throw new MetadataParseException(e.getMessage(), e.getLineNumber(), e.getColumnNumber(), e);
         }
@@ -71,4 +79,57 @@ private boolean isStrict(Map<String, ?> options) {
         Object value = (options != null) ? options.get(IS_STRICT) : null;
         return value == null || Boolean.parseBoolean(value.toString());
     }
+
+    /**
+     * Coordinate-shaped tokens read from this metadata (versions, plugin artifactIds and prefixes) get carried
+     * forward by callers as if they were already-validated path and coordinate components. Reject anything that
+     * would not itself be a valid coordinate component here, before it leaves this reader.
+     */
+    private static void validateMetadata(Metadata metadata) throws IOException {
+        if (metadata == null) {
+            return;
+        }
+
+        Versioning versioning = metadata.getVersioning();
+        if (versioning != null) {
+            validateToken("version", versioning.getRelease());
+            validateToken("version", versioning.getLatest());
+            for (String version : versioning.getVersions()) {
+                validateToken("version", version);
+            }
+            for (SnapshotVersion snapshotVersion : versioning.getSnapshotVersions()) {
+                validateToken("version", snapshotVersion.getVersion());
+            }
+            Snapshot snapshot = versioning.getSnapshot();
+            if (snapshot != null) {
+                validateToken("snapshot timestamp", snapshot.getTimestamp());
+            }
+        }
+
+        if (metadata.getPlugins() != null) {
+            for (Plugin plugin : metadata.getPlugins()) {
+                validateToken("plugin artifactId", plugin.getArtifactId());
+                validateToken("plugin prefix", plugin.getPrefix());
+            }
+        }
+    }
+
+    private static void validateToken(String field, String value) throws IOException {
+        if (value == null || value.isEmpty()) {
+            return;
+        }
+        boolean valid = !"..".equals(value);
+        if (valid) {
+            for (int i = 0; i < value.length(); i++) {
+                char c = value.charAt(i);
+                if (c == '/' || c == '\\' || c == ':' || Character.isISOControl(c)) {
+                    valid = false;
+                    break;
+                }
+            }
+        }
+        if (!valid) {
+            throw new IOException("Metadata contains an invalid " + field + ": '" + value + "'");
+        }
+    }
 }

maven-core/src/main/java/org/apache/maven/project/artifact/MavenMetadataSource.java

@@ -612,16 +612,19 @@ private ProjectRelocation retrieveRelocatedProject(Artifact artifact, MetadataRe
 
                     if (relocation != null) {
                         if (relocation.getGroupId() != null) {
+                            requireValidCoordinateComponent(relocation.getGroupId(), "groupId", artifact);
                             artifact.setGroupId(relocation.getGroupId());
                             relocatedArtifact = artifact;
                             project.setGroupId(relocation.getGroupId());
                         }
                         if (relocation.getArtifactId() != null) {
+                            requireValidCoordinateComponent(relocation.getArtifactId(), "artifactId", artifact);
                             artifact.setArtifactId(relocation.getArtifactId());
                             relocatedArtifact = artifact;
                             project.setArtifactId(relocation.getArtifactId());
                         }
                         if (relocation.getVersion() != null) {
+                            requireValidCoordinateComponent(relocation.getVersion(), "version", artifact);
                             // note: see MNG-3454. This causes a problem, but fixing it may break more.
                             artifact.setVersionRange(VersionRange.createFromVersion(relocation.getVersion()));
                             relocatedArtifact = artifact;
@@ -677,6 +680,34 @@ private ProjectRelocation retrieveRelocatedProject(Artifact artifact, MetadataRe
         return rel;
     }
 
+    /**
+     * Checks that a relocation coordinate component is usable as an artifact coordinate component before it
+     * is applied to the artifact and project. A component outside the coordinate character set is rejected so
+     * that only well-formed coordinates enter resolution.
+     */
+    private static void requireValidCoordinateComponent(String value, String component, Artifact artifact)
+            throws ArtifactMetadataRetrievalException {
+        if (isInvalidCoordinateComponent(value)) {
+            throw new ArtifactMetadataRetrievalException(
+                    "Invalid relocation " + component + " '" + value + "' for " + artifact.getId()
+                            + ": not a valid artifact coordinate component",
+                    null,
+                    artifact);
+        }
+    }
+
+    private static boolean isInvalidCoordinateComponent(String value) {
+        if ("..".equals(value) || value.contains("/") || value.contains("\\") || value.contains(":")) {
+            return true;
+        }
+        for (int i = 0; i < value.length(); i++) {
+            if (Character.isISOControl(value.charAt(i))) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     private ModelProblem hasMissingParentPom(ProjectBuildingException e) {
         if (e.getCause() instanceof ModelBuildingException) {
             ModelBuildingException mbe = (ModelBuildingException) e.getCause();

maven-core/src/test/java/org/apache/maven/artifact/repository/metadata/io/DefaultMetadataReaderTest.java

@@ -0,0 +1,67 @@
+/*
+ * 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 org.apache.maven.artifact.repository.metadata.io;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URISyntaxException;
+import java.util.Collections;
+
+import org.apache.maven.artifact.repository.metadata.Metadata;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class DefaultMetadataReaderTest {
+
+    private final DefaultMetadataReader reader = new DefaultMetadataReader();
+
+    private File resource(String name) throws URISyntaxException {
+        return new File(getClass().getResource(name).toURI());
+    }
+
+    @Test
+    public void testWellFormedMetadataParsesUnchanged() throws Exception {
+        Metadata metadata = reader.read(resource("well-formed-metadata.xml"), Collections.emptyMap());
+
+        assertEquals("org.apache.maven.its", metadata.getGroupId());
+        assertEquals("sample", metadata.getArtifactId());
+        assertEquals("1.1", metadata.getVersioning().getRelease());
+        assertEquals("1.1", metadata.getVersioning().getLatest());
+        assertEquals("maven-sample-plugin", metadata.getPlugins().get(0).getArtifactId());
+    }
+
+    @Test
+    public void testVersionContainingColonIsRejected() throws Exception {
+        File input = resource("invalid-version-token.xml");
+
+        IOException e = assertThrows(IOException.class, () -> reader.read(input, Collections.emptyMap()));
+        assertTrue(e.getMessage().contains("1.0:evil"));
+    }
+
+    @Test
+    public void testPluginArtifactIdContainingSlashIsRejected() throws Exception {
+        File input = resource("invalid-plugin-artifactid.xml");
+
+        IOException e = assertThrows(IOException.class, () -> reader.read(input, Collections.emptyMap()));
+        assertTrue(e.getMessage().contains("maven/sample-plugin"));
+    }
+}

maven-core/src/test/java/org/apache/maven/project/artifact/MavenMetadataSourceRelocationTest.java

@@ -0,0 +1,179 @@
+/*
+ * 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 org.apache.maven.project.artifact;
+
+import java.lang.reflect.Field;
+import java.util.Collections;
+
+import org.apache.maven.artifact.Artifact;
+import org.apache.maven.artifact.DefaultArtifact;
+import org.apache.maven.artifact.factory.ArtifactFactory;
+import org.apache.maven.artifact.handler.DefaultArtifactHandler;
+import org.apache.maven.artifact.metadata.ArtifactMetadataRetrievalException;
+import org.apache.maven.artifact.metadata.ResolutionGroup;
+import org.apache.maven.artifact.repository.ArtifactRepository;
+import org.apache.maven.artifact.repository.metadata.RepositoryMetadataManager;
+import org.apache.maven.bridge.MavenRepositorySystem;
+import org.apache.maven.model.DistributionManagement;
+import org.apache.maven.model.Relocation;
+import org.apache.maven.plugin.LegacySupport;
+import org.apache.maven.project.MavenProject;
+import org.apache.maven.project.ProjectBuilder;
+import org.apache.maven.project.ProjectBuildingRequest;
+import org.apache.maven.project.ProjectBuildingResult;
+import org.apache.maven.repository.legacy.metadata.DefaultMetadataResolutionRequest;
+import org.apache.maven.repository.legacy.metadata.MetadataResolutionRequest;
+import org.codehaus.plexus.logging.Logger;
+import org.eclipse.aether.RepositorySystemSession;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+/**
+ * Verifies that a relocation read from a resolved project's distribution management is validated before its
+ * components are applied to the artifact and project being resolved, exercising the real
+ * {@link MavenMetadataSource#retrieve(MetadataResolutionRequest)} code path with mocked collaborators (no
+ * on-disk artifact resolution, so this test does not depend on, or share, any module-level local repository).
+ */
+class MavenMetadataSourceRelocationTest {
+
+    private MavenMetadataSource newSource(ProjectBuilder projectBuilder) throws Exception {
+        MavenMetadataSource source = new MavenMetadataSource();
+
+        ArtifactFactory artifactFactory = mock(ArtifactFactory.class);
+        when(artifactFactory.createProjectArtifact(any(), any(), any(), any()))
+                .thenAnswer(invocation -> new DefaultArtifact(
+                        (String) invocation.getArgument(0),
+                        (String) invocation.getArgument(1),
+                        (String) invocation.getArgument(2),
+                        (String) invocation.getArgument(3),
+                        "pom",
+                        null,
+                        new DefaultArtifactHandler("pom")));
+
+        LegacySupport legacySupport = mock(LegacySupport.class);
+        RepositorySystemSession repositorySession = mock(RepositorySystemSession.class);
+        when(legacySupport.getRepositorySession()).thenReturn(repositorySession);
+        when(legacySupport.getSession()).thenReturn(null);
+
+        setField(source, "artifactFactory", artifactFactory);
+        setField(source, "repositorySystem", mock(MavenRepositorySystem.class));
+        setField(source, "repositoryMetadataManager", mock(RepositoryMetadataManager.class));
+        setField(source, "projectBuilder", projectBuilder);
+        setField(source, "logger", mock(Logger.class));
+        setField(source, "cache", mock(MavenMetadataCache.class));
+        setField(source, "legacySupport", legacySupport);
+
+        return source;
+    }
+
+    private static void setField(Object target, String name, Object value) throws Exception {
+        Field field = MavenMetadataSource.class.getDeclaredField(name);
+        field.setAccessible(true);
+        field.set(target, value);
+    }
+
+    private static Artifact newArtifact(String groupId, String artifactId, String version) {
+        return new DefaultArtifact(
+                groupId, artifactId, version, Artifact.SCOPE_COMPILE, "pom", null, new DefaultArtifactHandler("pom"));
+    }
+
+    private static MavenProject newProject(String groupId, String artifactId, String version, Relocation relocation) {
+        MavenProject project = new MavenProject();
+        project.setGroupId(groupId);
+        project.setArtifactId(artifactId);
+        project.setVersion(version);
+        if (relocation != null) {
+            DistributionManagement distMgmt = new DistributionManagement();
+            distMgmt.setRelocation(relocation);
+            project.setDistributionManagement(distMgmt);
+        }
+        return project;
+    }
+
+    private static MetadataResolutionRequest newRequest(Artifact artifact) {
+        MetadataResolutionRequest request = new DefaultMetadataResolutionRequest();
+        request.setArtifact(artifact);
+        request.setLocalRepository(mock(ArtifactRepository.class));
+        request.setRemoteRepositories(Collections.emptyList());
+        return request;
+    }
+
+    @Test
+    void testRelocationWithInvalidArtifactIdIsRejected() throws Exception {
+        Relocation relocation = new Relocation();
+        relocation.setArtifactId("a/b");
+
+        MavenProject relocatingProject = newProject("group", "original", "1.0", relocation);
+        MavenProject finalProject = newProject("group", "a/b", "1.0", null);
+
+        ProjectBuildingResult first = mock(ProjectBuildingResult.class);
+        when(first.getProject()).thenReturn(relocatingProject);
+        ProjectBuildingResult second = mock(ProjectBuildingResult.class);
+        when(second.getProject()).thenReturn(finalProject);
+
+        ProjectBuilder projectBuilder = mock(ProjectBuilder.class);
+        when(projectBuilder.build(any(Artifact.class), any(ProjectBuildingRequest.class)))
+                .thenReturn(first, second);
+
+        MavenMetadataSource source = newSource(projectBuilder);
+        Artifact artifact = newArtifact("group", "original", "1.0");
+        MetadataResolutionRequest request = newRequest(artifact);
+
+        ArtifactMetadataRetrievalException exception =
+                assertThrows(ArtifactMetadataRetrievalException.class, () -> source.retrieve(request));
+        assertEquals(true, exception.getMessage().contains("a/b"));
+        assertEquals(true, exception.getMessage().contains("artifactId"));
+    }
+
+    @Test
+    void testWellFormedRelocationIsApplied() throws Exception {
+        Relocation relocation = new Relocation();
+        relocation.setGroupId("group.moved");
+        relocation.setArtifactId("artifact-moved");
+        relocation.setVersion("2.0");
+
+        MavenProject relocatingProject = newProject("group", "original", "1.0", relocation);
+        MavenProject finalProject = newProject("group.moved", "artifact-moved", "2.0", null);
+
+        ProjectBuildingResult first = mock(ProjectBuildingResult.class);
+        when(first.getProject()).thenReturn(relocatingProject);
+        ProjectBuildingResult second = mock(ProjectBuildingResult.class);
+        when(second.getProject()).thenReturn(finalProject);
+
+        ProjectBuilder projectBuilder = mock(ProjectBuilder.class);
+        when(projectBuilder.build(any(Artifact.class), any(ProjectBuildingRequest.class)))
+                .thenReturn(first, second);
+
+        MavenMetadataSource source = newSource(projectBuilder);
+        Artifact artifact = newArtifact("group", "original", "1.0");
+        MetadataResolutionRequest request = newRequest(artifact);
+
+        ResolutionGroup result = source.retrieve(request);
+
+        assertEquals("group.moved", artifact.getGroupId());
+        assertEquals("artifact-moved", artifact.getArtifactId());
+        assertEquals("2.0", artifact.getVersion());
+        assertEquals(artifact, result.getRelocatedArtifact());
+    }
+}