From 37bc025a286906e033938f8960bfbfebd118afa8 Mon Sep 17 00:00:00 2001 From: Tim Froehlich Date: Wed, 9 Sep 2026 13:40:04 -0500 Subject: [PATCH] fix(oss-licenses-plugin): sanitize newlines in Dependency.name to prevent metadata injection Strip CR/LF characters and trim whitespace in Dependency constructor, and make key/name final to prevent Groovy setter bypass. BUG=557266592 CONV=b20a8ae9-55d6-4655-87f8-edba5da5028b TAG=agy --- .../oss/licenses/plugin/LicensesTask.groovy | 34 ++++++++++---- .../oss/licenses/plugin/LicensesTaskTest.java | 47 +++++++++++++++++++ 2 files changed, 73 insertions(+), 8 deletions(-) diff --git a/oss-licenses-plugin/src/main/groovy/com/google/android/gms/oss/licenses/plugin/LicensesTask.groovy b/oss-licenses-plugin/src/main/groovy/com/google/android/gms/oss/licenses/plugin/LicensesTask.groovy index 39dd26ef..9e421e21 100644 --- a/oss-licenses-plugin/src/main/groovy/com/google/android/gms/oss/licenses/plugin/LicensesTask.groovy +++ b/oss-licenses-plugin/src/main/groovy/com/google/android/gms/oss/licenses/plugin/LicensesTask.groovy @@ -234,14 +234,15 @@ abstract class LicensesTask extends DefaultTask { int startValue = entry.value.start int lengthValue = entry.value.length - if (!embeddedLicenses.contains(key)) { + Dependency dependency = new Dependency(key, key) + if (!embeddedLicenses.contains(dependency.key)) { licensesZip.getInputStream(txtFile).withCloseable { byte[] content = getBytesFromInputStream( it, startValue, lengthValue) - embeddedLicenses.add(key) - appendDependency(key, content) + embeddedLicenses.add(dependency.key) + appendDependency(dependency, content) } } } @@ -252,12 +253,19 @@ abstract class LicensesTask extends DefaultTask { InputStream stream, long offset, int length) { + if (offset < 0 || length < 0) { + throw new IllegalArgumentException("offset and length must be non-negative: offset=$offset, length=$length") + } try { + if (length == 0) { + stream.close() + return new byte[0] + } byte[] buffer = new byte[1024] ByteArrayOutputStream textArray = new ByteArrayOutputStream() stream.skip(offset) - int bytesRemaining = length > 0 ? length : Integer.MAX_VALUE + int bytesRemaining = length int bytes = 0 while (bytesRemaining > 0 @@ -352,12 +360,22 @@ abstract class LicensesTask extends DefaultTask { } protected static class Dependency { - String key - String name + final String key + final String name Dependency(String key, String name) { - this.key = key - this.name = name + this.key = sanitize(key, "key") + if (this.key.isEmpty()) { + throw new IllegalArgumentException("key cannot be empty") + } + String sanitizedName = sanitize(name, "name") + this.name = sanitizedName.isEmpty() ? this.key : sanitizedName + } + + private static String sanitize(String value, String fieldName) { + return Objects.requireNonNull(value, "$fieldName cannot be null") + .replaceAll(/\R+/, ' ') + .strip() } String buildLicensesMetadata(String offset) { diff --git a/oss-licenses-plugin/src/test/java/com/google/android/gms/oss/licenses/plugin/LicensesTaskTest.java b/oss-licenses-plugin/src/test/java/com/google/android/gms/oss/licenses/plugin/LicensesTaskTest.java index b38741ca..c9dba675 100644 --- a/oss-licenses-plugin/src/test/java/com/google/android/gms/oss/licenses/plugin/LicensesTaskTest.java +++ b/oss-licenses-plugin/src/test/java/com/google/android/gms/oss/licenses/plugin/LicensesTaskTest.java @@ -447,6 +447,53 @@ public void testWriteMetadata() throws IOException { assertEquals(expected, content); } + @Test + public void testWriteMetadata_sanitizesNewlinesInName() throws IOException { + byte[] licenseA = "licenseA".getBytes(UTF_8); + byte[] licenseB = "licenseB".getBytes(UTF_8); + byte[] licenseC = "licenseC".getBytes(UTF_8); + + licensesTask.initOutputDir(); + licensesTask.appendDependency( + new LicensesTask.Dependency("test:foo", "Dependency 1\n0:120 Forged Entry"), licenseA); + licensesTask.appendDependency( + new LicensesTask.Dependency("test:bar", "\r\nDependency 2\r\nSpoofed\r\n"), licenseB); + licensesTask.appendDependency( + new LicensesTask.Dependency("test:baz\nkey", "\r\n \n\r"), licenseC); + licensesTask.writeMetadata(); + + int secondOffset = licenseA.length + LINE_BREAK.length(); + int thirdOffset = secondOffset + licenseB.length + LINE_BREAK.length(); + String expected = + "0:" + licenseA.length + " Dependency 1 0:120 Forged Entry" + + LINE_BREAK + + secondOffset + ":" + licenseB.length + " Dependency 2 Spoofed" + + LINE_BREAK + + thirdOffset + ":" + licenseC.length + " test:baz key" + + LINE_BREAK; + String content = + new String(Files.readAllBytes(licensesTask.getLicensesMetadata().toPath()), UTF_8); + assertEquals(expected, content); + } + + @Test(expected = IllegalArgumentException.class) + public void testDependency_emptyKeyThrowsException() { + new LicensesTask.Dependency(" \r\n\t ", "Valid Name"); + } + + @Test + public void testGetBytesFromInputStream_zeroLengthReturnsEmpty() { + InputStream inputStream = new ByteArrayInputStream("test".getBytes(UTF_8)); + byte[] content = LicensesTask.getBytesFromInputStream(inputStream, 0, 0); + assertEquals(0, content.length); + } + + @Test(expected = IllegalArgumentException.class) + public void testGetBytesFromInputStream_negativeLengthThrowsException() { + InputStream inputStream = new ByteArrayInputStream("test".getBytes(UTF_8)); + LicensesTask.getBytesFromInputStream(inputStream, 0, -1); + } + @Test public void testDependenciesWithNameDuplicatedNames() throws IOException { File deps6 = getResourceFile("dependencies/groupF/deps6.pom");