You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by kw...@apache.org on 2026/10/15 07:32:20 UTC

(jackrabbit-filevault) 04/06: JCRVLT-860: reject backslash and control characters in PackageId validation

This is an automated email from the ASF dual-hosted git repository.

kwin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/jackrabbit-filevault.git

commit a9bc984f335b40c06f325ab0fec5d7d4ce4fb7e9
Author: Rishabh Kumar <di...@adobe.com>
AuthorDate: Mon Oct 5 08:50:36 2026 +0530

    JCRVLT-860: reject backslash and control characters in PackageId
    validation
    
    Co-authored-by: Copilot <22...@users.noreply.github.com>
---
 .../jackrabbit/vault/packaging/PackageId.java      |  7 ++++
 .../jackrabbit/vault/packaging/PackageIdTest.java  | 44 ++++++++++++++++++++++
 2 files changed, 51 insertions(+)

diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/PackageId.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/PackageId.java
index 757cb2ef..c8e537ef 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/PackageId.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/PackageId.java
@@ -560,6 +560,13 @@ public class PackageId implements Comparable<PackageId> {
                     nameStart = i;
                 }
                 trailingSpaces = false;
+            } else if (c == '\\' || Character.isISOControl(c)) {
+                // '\' is a path separator on Windows, and control characters have no
+                // legitimate use in a name — both would let an id escape the FS
+                // registry's home directory or spoof logs/names once used as a path
+                // segment (see PackageId's isValid() callers in FSPackageRegistry).
+                throw new IllegalArgumentException("'" + c + "' not allowed in name '" + jcrName
+                        + "'; remove the backslash" + " or control character from the group, name, or version");
             } else {
                 if (state == STATE_PREFIX_START) {
                     state = STATE_PREFIX; // prefix start
diff --git a/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/PackageIdTest.java b/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/PackageIdTest.java
index e9550338..45a68bc8 100644
--- a/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/PackageIdTest.java
+++ b/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/PackageIdTest.java
@@ -21,6 +21,8 @@ package org.apache.jackrabbit.vault.packaging;
 import org.junit.Test;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 
 /**
  * {@code PackageIdTest}...
@@ -212,4 +214,46 @@ public class PackageIdTest {
         assertEquals(p1.getGroup(), p2.getGroup());
         assertEquals(p1.getVersion(), p2.getVersion());
     }
+
+    @Test
+    public void testIsValidAcceptsRegularNames() {
+        // positive control: ordinary names/groups/versions must keep validating
+        // after the backslash/control-character fix is applied.
+        assertTrue(PackageId.isValid("my_company/sub-group", "my-package.name", "1.0.0-SNAPSHOT"));
+        assertTrue(PackageId.isValid("", "name", ""));
+        assertTrue(PackageId.isValid("group", "name", null));
+    }
+
+    @Test
+    public void testIsValidRejectsBackslashInName() {
+        // '..\..\evil' shaped ids must not validate: on Windows, Path.resolve
+        // treats '\' as a separator, so an id containing it can escape the
+        // FS registry's home directory.
+        assertFalse(PackageId.isValid("group", "..\\..\\evil", "1.0"));
+        assertFalse(PackageId.isValid("group", "evil\\name", "1.0"));
+    }
+
+    @Test
+    public void testIsValidRejectsBackslashInGroupSegment() {
+        // group segments are split on '/' and each segment re-validated, so a
+        // backslash hidden inside one segment must be rejected too.
+        assertFalse(PackageId.isValid("evil\\group", "name", "1.0"));
+        assertFalse(PackageId.isValid("outer/evil\\group", "name", "1.0"));
+    }
+
+    @Test
+    public void testIsValidRejectsBackslashInVersion() {
+        assertFalse(PackageId.isValid("group", "name", "..\\..\\evil"));
+    }
+
+    @Test
+    public void testIsValidRejectsControlCharacters() {
+        // control characters have no legitimate use in package ids and enable
+        // log/name spoofing; every id part must reject them.
+        assertFalse(PackageId.isValid("group", "name\u0000evil", "1.0"));
+        assertFalse(PackageId.isValid("group", "name\u001Fevil", "1.0"));
+        assertFalse(PackageId.isValid("group", "name\u007Fevil", "1.0"));
+        assertFalse(PackageId.isValid("evil\u0000group", "name", "1.0"));
+        assertFalse(PackageId.isValid("group", "name", "1.0\u0000evil"));
+    }
 }