You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by sj...@apache.org on 2026/09/06 15:42:25 UTC
(maven-install-plugin) branch maven-install-plugin-3.x updated: chore: harden embedded-POM trust boundary in install:install-file (3.x backport) (#447)
This is an automated email from the ASF dual-hosted git repository.
slawekjaranowski pushed a commit to branch maven-install-plugin-3.x
in repository https://gitbox.apache.org/repos/asf/maven-install-plugin.git
The following commit(s) were added to refs/heads/maven-install-plugin-3.x by this push:
new 5703691 chore: harden embedded-POM trust boundary in install:install-file (3.x backport) (#447)
5703691 is described below
commit 5703691341f42b2bc75c50df87cba020adb458b0
Author: Guillaume Nodet <gn...@gmail.com>
AuthorDate: Sun Sep 6 17:42:19 2026 +0200
chore: harden embedded-POM trust boundary in install:install-file (3.x backport) (#447)
* chore: harden embedded-POM trust boundary in install:install-file (3.x backport)
Backport of security hardening from master to the 3.x maintenance line.
Static analysis findings (no live exploit demonstrated):
- f001 (MEDIUM): Make coordinate adoption from embedded POM visible and
cross-checked — fail on multiple POM entries, validate entry-path vs
model GAV, log at INFO, warn before overwriting, validate packaging
- f002 (MEDIUM): Skip embedded POM when full CLI coordinates supplied;
cross-check operator-supplied coordinates against embedded POM
- f003 (LOW): Defense-in-depth DOCTYPE rejection for embedded POM XML
with encoding-aware screening (UTF-8/16/32, EBCDIC)
- f004 (LOW): Reject empty/dot-only coordinates and leading-dot groupIds;
containment-check install path against local repository root
Co-Authored-By: Claude Opus 4.6 <no...@anthropic.com>
* fix: update MINSTALL-52 IT to match INFO-level log messages
The embedded-POM trust hardening changed log.debug() to log.info()
for provenance visibility. Update the integration test assertions
to match the new log level and message format.
Co-Authored-By: Claude Opus 4.6 <no...@anthropic.com>
---------
Co-authored-by: Claude Opus 4.6 <no...@anthropic.com>
---
src/it/MINSTALL-52/verify.groovy | 4 +-
.../maven/plugins/install/InstallFileMojo.java | 352 +++++++++++++++++++--
.../maven/plugins/install/InstallFileMojoTest.java | 300 ++++++++++++++++++
3 files changed, 635 insertions(+), 21 deletions(-)
diff --git a/src/it/MINSTALL-52/verify.groovy b/src/it/MINSTALL-52/verify.groovy
index 617b5ae..4f28e28 100644
--- a/src/it/MINSTALL-52/verify.groovy
+++ b/src/it/MINSTALL-52/verify.groovy
@@ -22,5 +22,5 @@ assert new File( basedir, "../../local-repo/org/apache/maven/plugins/install/its
File buildLog = new File( basedir, 'build.log' )
assert buildLog.exists()
-assert buildLog.text.contains( "[DEBUG] Loading META-INF/maven/org.apache.maven.plugins.install.its/minstall52/pom.xml" )
-assert buildLog.text.contains( "[DEBUG] Using JAR embedded POM as pomFile" )
+assert buildLog.text.contains( "[INFO] Loading META-INF/maven/org.apache.maven.plugins.install.its/minstall52/pom.xml from" )
+assert buildLog.text.contains( "[INFO] Using JAR embedded POM as pomFile" )
diff --git a/src/main/java/org/apache/maven/plugins/install/InstallFileMojo.java b/src/main/java/org/apache/maven/plugins/install/InstallFileMojo.java
index 422b7d6..7981bec 100644
--- a/src/main/java/org/apache/maven/plugins/install/InstallFileMojo.java
+++ b/src/main/java/org/apache/maven/plugins/install/InstallFileMojo.java
@@ -25,14 +25,20 @@ import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
+import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.StandardCopyOption;
+import java.util.ArrayList;
+import java.util.List;
import java.util.Objects;
import java.util.function.Predicate;
import java.util.jar.JarEntry;
import java.util.jar.JarFile;
+import java.util.regex.Matcher;
import java.util.regex.Pattern;
+import java.util.stream.Collectors;
import org.apache.maven.execution.MavenSession;
import org.apache.maven.model.Model;
@@ -71,6 +77,11 @@ import static java.util.Objects.isNull;
@Mojo(name = "install-file", requiresProject = false, aggregator = true, threadSafe = true)
public class InstallFileMojo extends AbstractMojo {
private static final String LS = System.lineSeparator();
+ private static final String ILLEGAL_VERSION_CHARS = "\\/:\"<>|?*[](){},";
+
+ /** The {@code encoding} pseudo-attribute of an XML declaration. */
+ private static final Pattern ENCODING_PSEUDO_ATTR = Pattern.compile("encoding\\s*=\\s*[\"']([^\"']+)[\"']");
+
private final Logger log = LoggerFactory.getLogger(getClass());
private final RepositorySystem repositorySystem;
@@ -212,14 +223,29 @@ public class InstallFileMojo extends AbstractMojo {
File temporaryPom = null;
- if (pomFile == null) {
- temporaryPom = readingPomFromJarFile();
- if (!Boolean.TRUE.equals(generatePom)) {
+ if (pomFile != null) {
+ processModel(readModel(pomFile));
+ } else {
+ if (Boolean.TRUE.equals(generatePom)) {
+ // generatePom explicitly requested: skip embedded POM
+ } else if (groupId != null && artifactId != null && version != null && packaging != null) {
+ // The operator supplied the complete coordinates: do not let metadata embedded inside the
+ // (potentially untrusted) file silently become the authoritative POM for those coordinates.
+ // A minimal POM is generated below instead; use -DpomFile to install a curated POM.
+ log.info(
+ "Ignoring any POM embedded in {}: complete coordinates were supplied, so a minimal"
+ + " POM will be generated instead. Use -DpomFile to install a specific POM.",
+ file.getName());
+ } else {
+ // Coordinates that were supplied are cross-checked against the embedded POM inside
+ // readingPomFromJarFile(): a mismatch fails the build rather than installing the
+ // embedded POM verbatim at operator-chosen coordinates.
+ temporaryPom = readingPomFromJarFile();
pomFile = temporaryPom;
- log.debug("Using JAR embedded POM as pomFile");
+ if (pomFile != null) {
+ log.info("Using JAR embedded POM as pomFile");
+ }
}
- } else {
- processModel(readModel(pomFile));
}
if (isNull(groupId) || isNull(artifactId) || isNull(version) || isNull(packaging)) {
@@ -227,8 +253,9 @@ public class InstallFileMojo extends AbstractMojo {
+ "'version' and 'packaging' are required.");
}
- if (!isValidId(groupId) || !isValidId(artifactId) || !isValidVersion(version)) {
- throw new MojoExecutionException("The artifact information is not valid: uses invalid characters.");
+ if (!isValidGroupId(groupId) || !isValidId(artifactId) || !isValidId(packaging) || !isValidVersion(version)) {
+ throw new MojoExecutionException(
+ "The artifact information is not valid: uses invalid characters or empty/dot-only values.");
}
InstallRequest installRequest = new InstallRequest();
@@ -266,6 +293,17 @@ public class InstallFileMojo extends AbstractMojo {
throw new MojoFailureException("Cannot install artifact. " + "Artifact is already in the local repository."
+ LS + LS + "File in question is: " + file + LS);
}
+ if (artifactLocalFile.exists() && contentDiffers(artifactLocalFile, file)) {
+ log.warn(
+ "The local repository already contains {}:{}:{} at {} with different content:"
+ + " it will be overwritten by {}",
+ groupId,
+ artifactId,
+ version,
+ artifactLocalFile,
+ file);
+ }
+ verifyContainment(repositorySystemSession, artifactLocalFile);
if (!IS_POM_PACKAGING.test(packaging)) {
if (isNull(pomFile)) {
@@ -315,7 +353,15 @@ public class InstallFileMojo extends AbstractMojo {
try (JarFile jarFile = new JarFile(file)) {
- JarEntry pomEntry = jarFile.stream().filter(IS_POM_ENTRY).findAny().orElse(null);
+ List<JarEntry> pomEntries = jarFile.stream().filter(IS_POM_ENTRY).collect(Collectors.toList());
+ if (pomEntries.size() > 1) {
+ throw new MojoExecutionException("Found " + pomEntries.size() + " POM entries in " + file.getName()
+ + ": "
+ + pomEntries.stream().map(JarEntry::getName).collect(Collectors.joining(", "))
+ + ". Cannot decide which one to trust: use -DpomFile or supply explicit"
+ + " -DgroupId/-DartifactId/-Dversion/-Dpackaging instead.");
+ }
+ JarEntry pomEntry = pomEntries.isEmpty() ? null : pomEntries.get(0);
if (isNull(pomEntry)) {
// This means there is no entry which matches the "pom.xml"...(or in other words: not packaged by Maven)
@@ -323,12 +369,24 @@ public class InstallFileMojo extends AbstractMojo {
return null;
}
+ log.info("Loading {} from {}", pomEntry.getName(), file.getName());
+
Path tempPomFile = Files.createTempFile(base, ".pom");
Files.copy(jarFile.getInputStream(pomEntry), tempPomFile, StandardCopyOption.REPLACE_EXISTING);
- log.debug("Loading {}", pomEntry.getName());
- processModel(readModel(tempPomFile.toFile()));
+ rejectDoctype(tempPomFile, pomEntry.getName());
+ Model model = readModel(tempPomFile.toFile());
+ validateEmbeddedPomEntryPath(pomEntry.getName(), model);
+ crossCheckOperatorCoordinates(pomEntry.getName(), model);
+ processModel(model);
+ log.info(
+ "Using coordinates {}:{}:{}:{} from JAR embedded POM {}",
+ groupId,
+ artifactId,
+ version,
+ packaging,
+ pomEntry.getName());
return tempPomFile.toFile();
} catch (IOException e) {
@@ -356,6 +414,243 @@ public class InstallFileMojo extends AbstractMojo {
}
}
+ /**
+ * Defense-in-depth pre-screen for the JAR embedded POM: the entry is wire-origin, potentially
+ * attacker-authored XML, and it is handed to {@link MavenXpp3Reader} whose DTD and external-entity
+ * posture this plugin can neither configure nor guarantee across core versions. A valid POM never
+ * carries a DOCTYPE declaration, so any embedded POM containing one is rejected before it reaches
+ * the parser. The scan is encoding-aware: the bytes are decoded with the same charset an XML parser
+ * is required to autodetect (BOM / first-bytes / encoding pseudo-attribute, XML 1.0 Appendix F),
+ * so a UTF-16 or UTF-32 document cannot smuggle a NUL-interleaved DOCTYPE past a naive single-byte
+ * scan. This intentionally does not apply to the operator's own {@code -DpomFile}.
+ *
+ * @param embeddedPom the temporary file holding the extracted entry, must not be <code>null</code>
+ * @param entryName the name of the matched JAR entry, must not be <code>null</code>
+ * @throws MojoExecutionException if the content contains a DOCTYPE declaration or cannot be read
+ */
+ private void rejectDoctype(Path embeddedPom, String entryName) throws MojoExecutionException {
+ String content;
+ try {
+ byte[] bytes = Files.readAllBytes(embeddedPom);
+ content = new String(bytes, detectXmlCharset(bytes, entryName));
+ } catch (IOException e) {
+ throw new MojoExecutionException("Error reading embedded POM " + embeddedPom, e);
+ }
+ if (content.contains("<!DOCTYPE")) {
+ throw new MojoExecutionException("The POM embedded in " + file.getName() + " (" + entryName
+ + ") contains a DOCTYPE declaration. A POM must not declare a DOCTYPE; refusing to"
+ + " parse untrusted embedded XML with a DTD. Use -DpomFile or supply explicit"
+ + " coordinates instead.");
+ }
+ }
+
+ /**
+ * Detects the character encoding of raw XML bytes the way an XML parser is required to
+ * (XML 1.0 Appendix F): byte-order mark first, then the byte pattern of a leading {@code <?xml},
+ * then the {@code encoding} pseudo-attribute of the XML declaration, defaulting to UTF-8.
+ *
+ * @param bytes the raw entry bytes, must not be <code>null</code>
+ * @param entryName the name of the matched JAR entry, for error messages
+ * @return the detected charset, never <code>null</code>
+ * @throws MojoExecutionException if the detected/declared encoding is unsupported
+ */
+ private Charset detectXmlCharset(byte[] bytes, String entryName) throws MojoExecutionException {
+ if (bytes.length >= 4) {
+ int b0 = bytes[0] & 0xFF;
+ int b1 = bytes[1] & 0xFF;
+ int b2 = bytes[2] & 0xFF;
+ int b3 = bytes[3] & 0xFF;
+ // 4-byte BOMs and BOM-less "<?xml" patterns
+ if (b0 == 0x00 && b1 == 0x00 && b2 == 0xFE && b3 == 0xFF) {
+ return charsetOrFail("UTF-32BE", entryName);
+ }
+ if (b0 == 0xFF && b1 == 0xFE && b2 == 0x00 && b3 == 0x00) {
+ return charsetOrFail("UTF-32LE", entryName);
+ }
+ if (b0 == 0x00 && b1 == 0x00 && b2 == 0x00 && b3 == 0x3C) {
+ return charsetOrFail("UTF-32BE", entryName);
+ }
+ if (b0 == 0x3C && b1 == 0x00 && b2 == 0x00 && b3 == 0x00) {
+ return charsetOrFail("UTF-32LE", entryName);
+ }
+ if (b0 == 0x00 && b1 == 0x3C && b2 == 0x00 && b3 == 0x3F) {
+ return StandardCharsets.UTF_16BE;
+ }
+ if (b0 == 0x3C && b1 == 0x00 && b2 == 0x3F && b3 == 0x00) {
+ return StandardCharsets.UTF_16LE;
+ }
+ if (b0 == 0x4C && b1 == 0x6F && b2 == 0xA7 && b3 == 0x94) {
+ // "<?xm" in EBCDIC
+ String ebcdicPrefix =
+ new String(bytes, 0, Math.min(bytes.length, 1024), charsetOrFail("IBM037", entryName));
+ int ebcdicDeclEnd = ebcdicPrefix.indexOf("?>");
+ String ebcdicDecl = ebcdicDeclEnd >= 0 ? ebcdicPrefix.substring(0, ebcdicDeclEnd) : ebcdicPrefix;
+ Matcher ebcdicMatcher = ENCODING_PSEUDO_ATTR.matcher(ebcdicDecl);
+ if (ebcdicMatcher.find()) {
+ return charsetOrFail(ebcdicMatcher.group(1), entryName);
+ }
+ throw new MojoExecutionException("The POM embedded in " + file.getName() + " (" + entryName
+ + ") is EBCDIC-encoded but its XML declaration names no encoding, so the exact"
+ + " code page cannot be determined and it cannot be screened for a DOCTYPE"
+ + " declaration. Use -DpomFile or supply explicit coordinates instead.");
+ }
+ }
+ if (bytes.length >= 2) {
+ int b0 = bytes[0] & 0xFF;
+ int b1 = bytes[1] & 0xFF;
+ if (b0 == 0xFE && b1 == 0xFF) {
+ return StandardCharsets.UTF_16BE;
+ }
+ if (b0 == 0xFF && b1 == 0xFE) {
+ return StandardCharsets.UTF_16LE;
+ }
+ }
+ // ASCII-compatible family: honor a declared encoding if present
+ int offset =
+ bytes.length >= 3 && (bytes[0] & 0xFF) == 0xEF && (bytes[1] & 0xFF) == 0xBB && (bytes[2] & 0xFF) == 0xBF
+ ? 3
+ : 0;
+ String prefix = new String(bytes, offset, Math.min(bytes.length - offset, 1024), StandardCharsets.ISO_8859_1);
+ if (prefix.startsWith("<?xml")) {
+ int declEnd = prefix.indexOf("?>");
+ if (declEnd < 0) {
+ throw new MojoExecutionException("The POM embedded in " + file.getName() + " (" + entryName
+ + ") has an XML declaration that does not terminate within the sniffed prefix, so"
+ + " its declared encoding cannot be determined and it cannot be screened for a"
+ + " DOCTYPE declaration. Use -DpomFile or supply explicit coordinates instead.");
+ }
+ Matcher matcher = ENCODING_PSEUDO_ATTR.matcher(prefix.substring(0, declEnd));
+ if (matcher.find()) {
+ return charsetOrFail(matcher.group(1), entryName);
+ }
+ }
+ return StandardCharsets.UTF_8;
+ }
+
+ /**
+ * Resolves a detected or declared encoding name, failing closed if this JVM cannot decode it.
+ */
+ private Charset charsetOrFail(String name, String entryName) throws MojoExecutionException {
+ try {
+ return Charset.forName(name);
+ } catch (IllegalArgumentException e) {
+ throw new MojoExecutionException("The POM embedded in " + file.getName() + " (" + entryName
+ + ") uses an encoding this JVM cannot decode ('" + name + "'), so it cannot be screened"
+ + " for a DOCTYPE declaration. Use -DpomFile or supply explicit coordinates instead.");
+ }
+ }
+
+ /**
+ * Verifies that the {@code <groupId>/<artifactId>} components of the matched
+ * {@code META-INF/maven/<groupId>/<artifactId>/pom.xml} entry path agree with the effective
+ * coordinates declared by the embedded POM itself.
+ *
+ * @param entryName the name of the matched JAR entry, must not be <code>null</code>
+ * @param model the model parsed from that entry, must not be <code>null</code>
+ * @throws MojoExecutionException if the entry path does not match the model
+ */
+ private void validateEmbeddedPomEntryPath(String entryName, Model model) throws MojoExecutionException {
+ String[] segments = entryName.split("/");
+ // expected shape: META-INF/maven/<groupId>/<artifactId>/pom.xml
+ if (segments.length != 5) {
+ throw new MojoExecutionException("Unexpected embedded POM entry path '" + entryName + "' in "
+ + file.getName() + ": expected META-INF/maven/<groupId>/<artifactId>/pom.xml."
+ + " Use -DpomFile or supply explicit coordinates instead.");
+ }
+ String entryGroupId = segments[2];
+ String entryArtifactId = segments[3];
+ Parent parent = model.getParent();
+ String modelGroupId =
+ model.getGroupId() != null ? model.getGroupId() : (parent != null ? parent.getGroupId() : null);
+ String modelArtifactId = model.getArtifactId();
+ if (!entryGroupId.equals(modelGroupId) || !entryArtifactId.equals(modelArtifactId)) {
+ throw new MojoExecutionException("The POM embedded in " + file.getName() + " declares " + modelGroupId
+ + ":" + modelArtifactId + " but is packaged under entry path '" + entryName + "' ("
+ + entryGroupId + ":" + entryArtifactId + "). Refusing to adopt coordinates from inconsistent"
+ + " embedded metadata: use -DpomFile or supply explicit"
+ + " -DgroupId/-DartifactId/-Dversion/-Dpackaging.");
+ }
+ }
+
+ /**
+ * Cross-checks every operator-supplied coordinate (groupId, artifactId, version) against the
+ * effective values declared by the embedded POM and fails on any mismatch.
+ *
+ * @param entryName the name of the matched JAR entry, must not be <code>null</code>
+ * @param model the model parsed from that entry, must not be <code>null</code>
+ * @throws MojoExecutionException if an operator-supplied coordinate disagrees with the embedded POM
+ */
+ private void crossCheckOperatorCoordinates(String entryName, Model model) throws MojoExecutionException {
+ Parent parent = model.getParent();
+ String modelGroupId =
+ model.getGroupId() != null ? model.getGroupId() : (parent != null ? parent.getGroupId() : null);
+ String modelArtifactId = model.getArtifactId();
+ String modelVersion =
+ model.getVersion() != null ? model.getVersion() : (parent != null ? parent.getVersion() : null);
+ List<String> mismatches = new ArrayList<>();
+ if (groupId != null && !groupId.equals(modelGroupId)) {
+ mismatches.add("groupId: supplied '" + groupId + "' but embedded POM declares '" + modelGroupId + "'");
+ }
+ if (artifactId != null && !artifactId.equals(modelArtifactId)) {
+ mismatches.add(
+ "artifactId: supplied '" + artifactId + "' but embedded POM declares '" + modelArtifactId + "'");
+ }
+ if (version != null && !version.equals(modelVersion)) {
+ mismatches.add("version: supplied '" + version + "' but embedded POM declares '" + modelVersion + "'");
+ }
+ if (!mismatches.isEmpty()) {
+ throw new MojoExecutionException("The POM embedded in " + file.getName() + " (" + entryName
+ + ") does not match the supplied coordinates: " + String.join("; ", mismatches)
+ + ". Refusing to install a mismatching embedded POM at operator-chosen coordinates:"
+ + " use -DpomFile, supply all of -DgroupId/-DartifactId/-Dversion/-Dpackaging, or pass"
+ + " -DgeneratePom=true.");
+ }
+ }
+
+ /**
+ * Returns {@code true} if the two files exist with different content, or cannot be compared
+ * (fail-closed for warning purposes). Uses byte-by-byte comparison since {@code Files.mismatch()}
+ * is not available on Java 8.
+ */
+ private boolean contentDiffers(File existing, File candidate) {
+ try {
+ if (existing.length() != candidate.length()) {
+ return true;
+ }
+ byte[] existingBytes = Files.readAllBytes(existing.toPath());
+ byte[] candidateBytes = Files.readAllBytes(candidate.toPath());
+ for (int i = 0; i < existingBytes.length; i++) {
+ if (existingBytes[i] != candidateBytes[i]) {
+ return true;
+ }
+ }
+ return false;
+ } catch (IOException e) {
+ return true;
+ }
+ }
+
+ /**
+ * Defense in depth: verifies that the composed layout path stays inside the local repository.
+ */
+ private void verifyContainment(RepositorySystemSession repoSession, File artifactLocalFile)
+ throws MojoExecutionException {
+ File repositoryRoot = repoSession.getLocalRepository().getBasedir().getAbsoluteFile();
+ File resolvedInstallPath = artifactLocalFile.getAbsoluteFile();
+ try {
+ String repoCanonical = repositoryRoot.getCanonicalPath();
+ String installCanonical = resolvedInstallPath.getCanonicalPath();
+ if (!installCanonical.startsWith(repoCanonical + File.separator)
+ && !installCanonical.equals(repoCanonical)) {
+ throw new MojoExecutionException("The artifact coordinates " + groupId + ":" + artifactId + ":"
+ + version + " resolve to a path outside the local repository: " + resolvedInstallPath
+ + " is not under " + repositoryRoot);
+ }
+ } catch (IOException e) {
+ throw new MojoExecutionException("Failed to resolve canonical path for containment check", e);
+ }
+ }
+
/**
* Populates missing mojo parameters from the specified POM.
*
@@ -447,12 +742,15 @@ public class InstallFileMojo extends AbstractMojo {
// these below should be shared (duplicated in m-install-p, m-deploy-p)
/**
- * Returns {@code true} if passed in string is "valid Maven ID" (groupId or artifactId).
+ * Returns {@code true} if passed in string is "valid Maven ID" (artifactId or packaging): non-empty,
+ * not consisting solely of {@code '.'} characters (so it can never form a {@code .}/{@code ..} path
+ * segment in the local repository layout), and using only allowed characters.
*/
private boolean isValidId(String id) {
- if (id == null) {
+ if (id == null || id.isEmpty()) {
return false;
}
+ boolean seenNonDot = false;
for (int i = 0; i < id.length(); i++) {
char c = id.charAt(i);
if (!(c >= 'a' && c <= 'z'
@@ -463,24 +761,40 @@ public class InstallFileMojo extends AbstractMojo {
|| c == '.')) {
return false;
}
+ if (c != '.') {
+ seenNonDot = true;
+ }
}
- return true;
+ return seenNonDot;
}
- private static final String ILLEGAL_VERSION_CHARS = "\\/:\"<>|?*[](){},";
+ /**
+ * Returns {@code true} if passed in string is a valid Maven groupId: a valid ID whose dot-separated
+ * segments are all non-empty. Leading, trailing or consecutive dots would produce empty path segments
+ * after the dots-to-directories transform of the local repository layout.
+ */
+ private boolean isValidGroupId(String groupId) {
+ return isValidId(groupId) && !groupId.startsWith(".") && !groupId.endsWith(".") && !groupId.contains("..");
+ }
/**
- * Returns {@code true} if passed in string is "valid Maven (simple. non range, expression, etc) version".
+ * Returns {@code true} if passed in string is "valid Maven (simple. non range, expression, etc) version":
+ * non-empty, not consisting solely of {@code '.'} characters, and free of illegal characters.
*/
private boolean isValidVersion(String version) {
- if (version == null) {
+ if (version == null || version.isEmpty()) {
return false;
}
+ boolean seenNonDot = false;
for (int i = version.length() - 1; i >= 0; i--) {
- if (ILLEGAL_VERSION_CHARS.indexOf(version.charAt(i)) >= 0) {
+ char c = version.charAt(i);
+ if (ILLEGAL_VERSION_CHARS.indexOf(c) >= 0) {
return false;
}
+ if (c != '.') {
+ seenNonDot = true;
+ }
}
- return true;
+ return seenNonDot;
}
}
diff --git a/src/test/java/org/apache/maven/plugins/install/InstallFileMojoTest.java b/src/test/java/org/apache/maven/plugins/install/InstallFileMojoTest.java
index 52a048a..1127cbe 100644
--- a/src/test/java/org/apache/maven/plugins/install/InstallFileMojoTest.java
+++ b/src/test/java/org/apache/maven/plugins/install/InstallFileMojoTest.java
@@ -22,8 +22,12 @@ import javax.inject.Inject;
import java.io.File;
import java.io.Reader;
+import java.lang.reflect.Field;
+import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
+import java.util.jar.JarEntry;
+import java.util.jar.JarOutputStream;
import org.apache.maven.api.plugin.testing.Basedir;
import org.apache.maven.api.plugin.testing.InjectMojo;
@@ -33,6 +37,7 @@ import org.apache.maven.api.plugin.testing.MojoTest;
import org.apache.maven.execution.MavenSession;
import org.apache.maven.model.Model;
import org.apache.maven.model.io.xpp3.MavenXpp3Reader;
+import org.apache.maven.plugin.MojoExecutionException;
import org.codehaus.plexus.util.FileUtils;
import org.codehaus.plexus.util.xml.XmlStreamReader;
import org.junit.jupiter.api.BeforeEach;
@@ -40,6 +45,8 @@ import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
/**
@@ -209,4 +216,297 @@ public class InstallFileMojoTest {
assertEquals(4, FileUtils.getFiles(localRepo, null, null).size());
}
+
+ // -------------------------------------------------------------------------
+ // Security hardening tests (f001, f002, f003, f004)
+ // -------------------------------------------------------------------------
+
+ @Test
+ @InjectMojo(goal = "install-file")
+ @MojoParameter(name = "file", value = "${basedir}/target/maven-install-test-1.0-SNAPSHOT.jar")
+ void adoptsCoordinatesFromConsistentEmbeddedPom(InstallFileMojo mojo) throws Exception {
+ assertNotNull(mojo);
+ // Clear class-level coordinates so the embedded POM is the sole source
+ setMojoField(mojo, "groupId", null);
+ setMojoField(mojo, "artifactId", null);
+ setMojoField(mojo, "version", null);
+ Path jar = createJarWithEntries(
+ pomXml("org.example", "embedded-lib", "1.0"), "META-INF/maven/org.example/embedded-lib/pom.xml");
+ setMojoField(mojo, "file", jar.toFile());
+
+ mojo.execute();
+
+ assertEquals("org.example", getMojoField(mojo, "groupId"));
+ assertEquals("embedded-lib", getMojoField(mojo, "artifactId"));
+ assertEquals("1.0", getMojoField(mojo, "version"));
+ }
+
+ @Test
+ @InjectMojo(goal = "install-file")
+ @MojoParameter(name = "file", value = "${basedir}/target/maven-install-test-1.0-SNAPSHOT.jar")
+ void rejectsEmbeddedPomWithMismatchedEntryPath(InstallFileMojo mojo) throws Exception {
+ assertNotNull(mojo);
+ setMojoField(mojo, "groupId", null);
+ setMojoField(mojo, "artifactId", null);
+ setMojoField(mojo, "version", null);
+ // Decoy entry path does not match the coordinates the embedded POM declares
+ Path jar = createJarWithEntries(
+ pomXml("org.apache.maven.plugins", "maven-clean-plugin", "3.4.0"),
+ "META-INF/maven/org.evil/decoy/pom.xml");
+ setMojoField(mojo, "file", jar.toFile());
+
+ MojoExecutionException e = assertThrows(MojoExecutionException.class, mojo::execute);
+ assertTrue(e.getMessage().contains("entry path"), e.getMessage());
+ }
+
+ @Test
+ @InjectMojo(goal = "install-file")
+ @MojoParameter(name = "file", value = "${basedir}/target/maven-install-test-1.0-SNAPSHOT.jar")
+ void rejectsJarWithMultipleEmbeddedPoms(InstallFileMojo mojo) throws Exception {
+ assertNotNull(mojo);
+ setMojoField(mojo, "groupId", null);
+ setMojoField(mojo, "artifactId", null);
+ setMojoField(mojo, "version", null);
+ Path jar = createJarWithEntries(
+ pomXml("org.example", "embedded-lib", "1.0"),
+ "META-INF/maven/org.example/embedded-lib/pom.xml",
+ "META-INF/maven/org.other/other-lib/pom.xml");
+ setMojoField(mojo, "file", jar.toFile());
+
+ MojoExecutionException e = assertThrows(MojoExecutionException.class, mojo::execute);
+ assertTrue(e.getMessage().contains("POM entries"), e.getMessage());
+ }
+
+ @Test
+ @InjectMojo(goal = "install-file")
+ @MojoParameter(name = "packaging", value = "jar!")
+ @MojoParameter(name = "file", value = "${basedir}/target/maven-install-test-1.0-SNAPSHOT.jar")
+ void invalidPackagingRejected(InstallFileMojo mojo) {
+ assertNotNull(mojo);
+
+ MojoExecutionException e = assertThrows(MojoExecutionException.class, mojo::execute);
+ assertTrue(e.getMessage().contains("not valid"), e.getMessage());
+ }
+
+ @Test
+ @InjectMojo(goal = "install-file")
+ @MojoParameter(name = "packaging", value = "jar")
+ @MojoParameter(name = "file", value = "${basedir}/target/maven-install-test-1.0-SNAPSHOT.jar")
+ void embeddedPomIgnoredWhenFullCoordinatesSupplied(InstallFileMojo mojo) throws Exception {
+ assertNotNull(mojo);
+ // Hostile embedded POM carrying a foreign GAV and an injected dependency
+ String evilPom = "<project>" + "<modelVersion>4.0.0</modelVersion>"
+ + "<groupId>com.evil</groupId>"
+ + "<artifactId>injected</artifactId>"
+ + "<version>9.9</version>"
+ + "<packaging>jar</packaging>"
+ + "<dependencies><dependency>"
+ + "<groupId>com.evil</groupId><artifactId>backdoor</artifactId><version>1.0</version>"
+ + "</dependency></dependencies>"
+ + "</project>";
+ Path jar = createJarWithEntries(evilPom, "META-INF/maven/com.evil/injected/pom.xml");
+ setMojoField(mojo, "file", jar.toFile());
+
+ // Full coordinates supplied via annotations: the embedded POM must be ignored
+ mojo.execute();
+
+ // Verify the installed POM is the generated one, not the evil embedded one
+ File installedPom = new File(
+ localRepo,
+ "org/apache/maven/test/maven-install-file-test/1.0-SNAPSHOT/maven-install-file-test-1.0-SNAPSHOT.pom");
+ assertTrue(installedPom.exists(), "Generated POM should be installed");
+ try (Reader reader = new XmlStreamReader(installedPom)) {
+ Model model = new MavenXpp3Reader().read(reader);
+ assertEquals("org.apache.maven.test", model.getGroupId());
+ assertEquals("maven-install-file-test", model.getArtifactId());
+ assertEquals("1.0-SNAPSHOT", model.getVersion());
+ assertTrue(model.getDependencies().isEmpty(), "Generated POM should have no dependencies");
+ }
+ }
+
+ @Test
+ @InjectMojo(goal = "install-file")
+ @MojoParameter(name = "file", value = "${basedir}/target/maven-install-test-1.0-SNAPSHOT.jar")
+ void mismatchedEmbeddedPomRejectedWhenPackagingOmitted(InstallFileMojo mojo) throws Exception {
+ assertNotNull(mojo);
+ // Packaging omitted (not set via annotation), so the embedded POM is consulted.
+ // The attacker POM is internally consistent (entry path matches its own declared GAV)
+ // but disagrees with the supplied coordinates (g/a/v from class level).
+ Path jar =
+ createJarWithEntries(pomXml("com.evil", "injected", "9.9"), "META-INF/maven/com.evil/injected/pom.xml");
+ setMojoField(mojo, "file", jar.toFile());
+
+ MojoExecutionException e = assertThrows(MojoExecutionException.class, mojo::execute);
+ assertTrue(e.getMessage().contains("does not match the supplied coordinates"), e.getMessage());
+ }
+
+ @Test
+ @InjectMojo(goal = "install-file")
+ @MojoParameter(name = "file", value = "${basedir}/target/maven-install-test-1.0-SNAPSHOT.jar")
+ void matchingEmbeddedPomAcceptedWhenPackagingOmitted(InstallFileMojo mojo) throws Exception {
+ assertNotNull(mojo);
+ // Packaging omitted; the embedded POM agrees with every supplied coordinate, so it may
+ // be used and contribute the missing packaging
+ Path jar = createJarWithEntries(
+ pomXml("org.apache.maven.test", "maven-install-file-test", "1.0-SNAPSHOT"),
+ "META-INF/maven/org.apache.maven.test/maven-install-file-test/pom.xml");
+ setMojoField(mojo, "file", jar.toFile());
+
+ mojo.execute();
+
+ assertEquals("jar", getMojoField(mojo, "packaging"));
+ }
+
+ @Test
+ @InjectMojo(goal = "install-file")
+ @MojoParameter(name = "file", value = "${basedir}/target/maven-install-test-1.0-SNAPSHOT.jar")
+ void rejectsEmbeddedPomWithDoctype(InstallFileMojo mojo) throws Exception {
+ assertNotNull(mojo);
+ setMojoField(mojo, "groupId", null);
+ setMojoField(mojo, "artifactId", null);
+ setMojoField(mojo, "version", null);
+ String xxePom = "<?xml version=\"1.0\"?>"
+ + "<!DOCTYPE project [<!ENTITY xxe SYSTEM \"file:///etc/passwd\">]>"
+ + pomXml("org.example", "embedded-lib", "1.0");
+ Path jar = createJarWithEntries(xxePom, "META-INF/maven/org.example/embedded-lib/pom.xml");
+ setMojoField(mojo, "file", jar.toFile());
+
+ MojoExecutionException e = assertThrows(MojoExecutionException.class, mojo::execute);
+ assertTrue(e.getMessage().contains("DOCTYPE"), e.getMessage());
+ }
+
+ @Test
+ @InjectMojo(goal = "install-file")
+ @MojoParameter(name = "file", value = "${basedir}/target/maven-install-test-1.0-SNAPSHOT.jar")
+ void rejectsEmbeddedPomWithDoctypeUtf16Le(InstallFileMojo mojo) throws Exception {
+ assertNotNull(mojo);
+ setMojoField(mojo, "groupId", null);
+ setMojoField(mojo, "artifactId", null);
+ setMojoField(mojo, "version", null);
+ String xxePom = "<?xml version=\"1.0\"?>"
+ + "<!DOCTYPE project [<!ENTITY xxe SYSTEM \"file:///etc/passwd\">]>"
+ + pomXml("org.example", "embedded-lib", "1.0");
+ // BOM-prefixed UTF-16LE: interleaved NUL bytes defeat a single-byte substring scan
+ byte[] bytes = ("" + xxePom).getBytes(StandardCharsets.UTF_16LE);
+ Path jar = createJarWithEntryBytes(bytes, "META-INF/maven/org.example/embedded-lib/pom.xml");
+ setMojoField(mojo, "file", jar.toFile());
+
+ MojoExecutionException e = assertThrows(MojoExecutionException.class, mojo::execute);
+ assertTrue(e.getMessage().contains("DOCTYPE"), e.getMessage());
+ }
+
+ @Test
+ @InjectMojo(goal = "install-file")
+ @MojoParameter(name = "file", value = "${basedir}/target/maven-install-test-1.0-SNAPSHOT.jar")
+ void rejectsEmbeddedPomWithDoctypeUtf16Be(InstallFileMojo mojo) throws Exception {
+ assertNotNull(mojo);
+ setMojoField(mojo, "groupId", null);
+ setMojoField(mojo, "artifactId", null);
+ setMojoField(mojo, "version", null);
+ String xxePom = "<?xml version=\"1.0\"?>"
+ + "<!DOCTYPE project [<!ENTITY xxe SYSTEM \"file:///etc/passwd\">]>"
+ + pomXml("org.example", "embedded-lib", "1.0");
+ // BOM-prefixed UTF-16BE
+ byte[] bytes = ("" + xxePom).getBytes(StandardCharsets.UTF_16BE);
+ Path jar = createJarWithEntryBytes(bytes, "META-INF/maven/org.example/embedded-lib/pom.xml");
+ setMojoField(mojo, "file", jar.toFile());
+
+ MojoExecutionException e = assertThrows(MojoExecutionException.class, mojo::execute);
+ assertTrue(e.getMessage().contains("DOCTYPE"), e.getMessage());
+ }
+
+ @Test
+ @InjectMojo(goal = "install-file")
+ @MojoParameter(name = "groupId", value = ".tmp.evil")
+ @MojoParameter(name = "packaging", value = "jar")
+ @MojoParameter(name = "file", value = "${basedir}/target/maven-install-test-1.0-SNAPSHOT.jar")
+ void groupIdWithLeadingDotRejected(InstallFileMojo mojo) {
+ assertNotNull(mojo);
+
+ MojoExecutionException e = assertThrows(MojoExecutionException.class, mojo::execute);
+ assertTrue(e.getMessage().contains("not valid"), e.getMessage());
+ }
+
+ @Test
+ @InjectMojo(goal = "install-file")
+ @MojoParameter(name = "groupId", value = "org..evil")
+ @MojoParameter(name = "packaging", value = "jar")
+ @MojoParameter(name = "file", value = "${basedir}/target/maven-install-test-1.0-SNAPSHOT.jar")
+ void groupIdWithConsecutiveDotsRejected(InstallFileMojo mojo) {
+ assertNotNull(mojo);
+
+ MojoExecutionException e = assertThrows(MojoExecutionException.class, mojo::execute);
+ assertTrue(e.getMessage().contains("not valid"), e.getMessage());
+ }
+
+ @Test
+ @InjectMojo(goal = "install-file")
+ @MojoParameter(name = "artifactId", value = "..")
+ @MojoParameter(name = "packaging", value = "jar")
+ @MojoParameter(name = "file", value = "${basedir}/target/maven-install-test-1.0-SNAPSHOT.jar")
+ void dotDotArtifactIdRejected(InstallFileMojo mojo) {
+ assertNotNull(mojo);
+
+ MojoExecutionException e = assertThrows(MojoExecutionException.class, mojo::execute);
+ assertTrue(e.getMessage().contains("not valid"), e.getMessage());
+ }
+
+ @Test
+ @InjectMojo(goal = "install-file")
+ @MojoParameter(name = "version", value = "..")
+ @MojoParameter(name = "packaging", value = "jar")
+ @MojoParameter(name = "file", value = "${basedir}/target/maven-install-test-1.0-SNAPSHOT.jar")
+ void dotDotVersionRejected(InstallFileMojo mojo) {
+ assertNotNull(mojo);
+
+ MojoExecutionException e = assertThrows(MojoExecutionException.class, mojo::execute);
+ assertTrue(e.getMessage().contains("not valid"), e.getMessage());
+ }
+
+ // -------------------------------------------------------------------------
+ // Helpers
+ // -------------------------------------------------------------------------
+
+ private static Path createJarWithEntries(String pomContent, String... entryNames) throws Exception {
+ Path jar = Files.createTempFile("maven-install-file-test", ".jar");
+ try (JarOutputStream jos = new JarOutputStream(Files.newOutputStream(jar))) {
+ for (String entryName : entryNames) {
+ jos.putNextEntry(new JarEntry(entryName));
+ jos.write(pomContent.getBytes(StandardCharsets.UTF_8));
+ jos.closeEntry();
+ }
+ }
+ return jar;
+ }
+
+ private static Path createJarWithEntryBytes(byte[] pomContent, String entryName) throws Exception {
+ Path jar = Files.createTempFile("maven-install-file-test", ".jar");
+ try (JarOutputStream jos = new JarOutputStream(Files.newOutputStream(jar))) {
+ jos.putNextEntry(new JarEntry(entryName));
+ jos.write(pomContent);
+ jos.closeEntry();
+ }
+ return jar;
+ }
+
+ private static String pomXml(String groupId, String artifactId, String version) {
+ return "<project>" + "<modelVersion>4.0.0</modelVersion>"
+ + "<groupId>" + groupId + "</groupId>"
+ + "<artifactId>" + artifactId + "</artifactId>"
+ + "<version>" + version + "</version>"
+ + "<packaging>jar</packaging>"
+ + "</project>";
+ }
+
+ private static void setMojoField(Object mojo, String fieldName, Object value) throws Exception {
+ Field f = mojo.getClass().getDeclaredField(fieldName);
+ f.setAccessible(true);
+ f.set(mojo, value);
+ }
+
+ private static Object getMojoField(Object mojo, String fieldName) throws Exception {
+ Field f = mojo.getClass().getDeclaredField(fieldName);
+ f.setAccessible(true);
+ return f.get(mojo);
+ }
}