always validate content paths

ocfl-java-core/src/main/java/edu/wisc/library/ocfl/core/OcflRepositoryBuilder.java

@@ -99,7 +99,7 @@ public OcflRepositoryBuilder() {
                 .maximumSize(512).build());
         inventoryMapper = InventoryMapper.defaultMapper();
         logicalPathMapper = LogicalPathMappers.directMapper();
-        contentPathConstraintProcessor = ContentPathConstraints.none();
+        contentPathConstraintProcessor = ContentPathConstraints.minimal();
         unsupportedBehavior = UnsupportedExtensionBehavior.FAIL;
         ignoreUnsupportedExtensions = Collections.emptySet();
         verifyStaging = true;
@@ -262,7 +262,7 @@ public OcflRepositoryBuilder logicalPathMapper(LogicalPathMapper logicalPathMapp
      *     <li>{@link ContentPathConstraints#windows()}</li>
      *     <li>{@link ContentPathConstraints#cloud()}</li>
      *     <li>{@link ContentPathConstraints#all()}</li>
-     *     <li>{@link ContentPathConstraints#none()}</li>
+     *     <li>{@link ContentPathConstraints#minimal()}</li>
      * </ul>
      *
      * <p>Constraints should be applied that target filesystems that are NOT the local filesystem. The local filesystem

ocfl-java-core/src/main/java/edu/wisc/library/ocfl/core/inventory/InventoryMapper.java

@@ -33,6 +33,8 @@
 import edu.wisc.library.ocfl.api.util.Enforce;
 import edu.wisc.library.ocfl.core.model.Inventory;
 import edu.wisc.library.ocfl.core.model.RevisionNum;
+import edu.wisc.library.ocfl.core.path.constraint.ContentPathConstraintProcessor;
+import edu.wisc.library.ocfl.core.path.constraint.ContentPathConstraints;
 import edu.wisc.library.ocfl.core.util.ObjectMappers;
 
 import java.io.BufferedInputStream;
@@ -43,6 +45,7 @@
 import java.nio.file.NoSuchFileException;
 import java.nio.file.Path;
 import java.security.DigestInputStream;
+import java.util.Collection;
 
 /**
  * Wrapper around Jackson's ObjectMapper for serializing and deserializing Inventories. The ObjectMapper setup is a finicky
@@ -51,6 +54,7 @@
 public class InventoryMapper {
 
     private final ObjectMapper objectMapper;
+    private final ContentPathConstraintProcessor contentPathConstraints;
 
     /**
      * Creates an InventoryMapper that will pretty print JSON files. This should be used when you value human readability
@@ -79,6 +83,7 @@ public static InventoryMapper defaultMapper() {
      */
     public InventoryMapper(ObjectMapper objectMapper) {
         this.objectMapper = Enforce.notNull(objectMapper, "objectMapper cannot be null");
+        this.contentPathConstraints = ContentPathConstraints.minimal();
     }
 
     public void write(Path destination, Inventory inventory) {
@@ -150,14 +155,21 @@ private Inventory readInternal(boolean mutableHead,
                                    String objectRootPath,
                                    ReadResult readResult) {
         try {
-            return objectMapper.reader(
+            Inventory inventory = objectMapper.reader(
                     new InjectableValues.Std()
                             .addValue("revisionNum", revisionNum)
                             .addValue("mutableHead", mutableHead)
                             .addValue("objectRootPath", objectRootPath)
                             .addValue("inventoryDigest", readResult.digest))
                     .forType(Inventory.class)
                     .readValue(readResult.bytes);
+
+            // Ensure that all content paths are valid to avoid security problems due to malicious inventories
+            inventory.getManifest().values().stream()
+                    .flatMap(Collection::stream)
+                    .forEach(contentPathConstraints::apply);
+
+            return inventory;
         } catch (IOException e) {
             throw new OcflIOException(e);
         }

ocfl-java-core/src/main/java/edu/wisc/library/ocfl/core/path/ContentPathMapper.java

@@ -60,7 +60,7 @@ public static class Builder {
 
         public Builder() {
             logicalPathMapper = new DirectLogicalPathMapper();
-            contentPathConstraintProcessor = ContentPathConstraints.none();
+            contentPathConstraintProcessor = ContentPathConstraints.minimal();
         }
 
         public Builder logicalPathMapper(LogicalPathMapper logicalPathMapper) {

ocfl-java-core/src/main/java/edu/wisc/library/ocfl/core/path/constraint/ContentPathConstraintProcessor.java

@@ -36,4 +36,13 @@ public interface ContentPathConstraintProcessor {
      */
     void apply(String contentPath, String storagePath);
 
+    /**
+     * Applies the configured path constrains to the content path and storage path. If any constraints fail,
+     * a {@link edu.wisc.library.ocfl.api.exception.PathConstraintException} is thrown.
+     *
+     * @param contentPath the content path relative a version's content directory
+     * @throws edu.wisc.library.ocfl.api.exception.PathConstraintException when a constraint fails
+     */
+    void apply(String contentPath);
+
 }

ocfl-java-core/src/main/java/edu/wisc/library/ocfl/core/path/constraint/ContentPathConstraints.java

@@ -178,11 +178,19 @@ public static ContentPathConstraintProcessor all() {
     }
 
     /**
-     * Constructs a ContentPathConstraintProcessor that does no special validation.
+     * Constructs a ContentPathConstraintProcessor that does the minimal content path validations as required
+     * by the OCFL spec.
+     *
+     * <ul>
+     *     <li>Cannot have a leading OR trailing /</li>
+     *     <li>Cannot contain the following filenames: '.', '..'</li>
+     *     <li>Cannot contain an empty filename</li>
+     *     <li>Windows only: Cannot contain a \</li>
+     * </ul>
      *
      * @return ContentPathConstraintProcessor
      */
-    public static ContentPathConstraintProcessor none() {
+    public static ContentPathConstraintProcessor minimal() {
         return DefaultContentPathConstraintProcessor.builder().build();
     }
 

ocfl-java-core/src/main/java/edu/wisc/library/ocfl/core/path/constraint/DefaultContentPathConstraintProcessor.java

@@ -26,6 +26,7 @@
 
 import edu.wisc.library.ocfl.api.util.Enforce;
 
+import java.nio.file.FileSystems;
 import java.util.regex.Pattern;
 
 /**
@@ -115,9 +116,12 @@ public DefaultContentPathConstraintProcessor(PathConstraintProcessor storagePath
         this.storagePathConstraintProcessor = Enforce.notNull(storagePathConstraintProcessor, "storagePathConstraintProcessor cannot be null");
         this.contentPathConstraintProcessor = Enforce.notNull(contentPathConstraintProcessor, "contentPathConstraintProcessor cannot be null");
 
+        if (filesystemUsesBackslashSeparator()) {
+            this.contentPathConstraintProcessor.prependCharConstraint(BACKSLASH_CONSTRAINT);
+        }
+
         // Add the required content path constraints to the beginning of the content path constraint processor constraint list
         this.contentPathConstraintProcessor
-                .prependCharConstraint(BACKSLASH_CONSTRAINT)
                 .prependFileNameConstraint(DOT_CONSTRAINT)
                 .prependFileNameConstraint(NON_EMPTY_CONSTRAINT)
                 .prependPathConstraint(TRAILING_SLASH_CONSTRAINT)
@@ -133,4 +137,19 @@ public void apply(String contentPath, String storagePath) {
         contentPathConstraintProcessor.apply(contentPath);
     }
 
+    /**
+     * {@inheritDoc}
+     */
+    @Override
+    public void apply(String contentPath) {
+        contentPathConstraintProcessor.apply(contentPath);
+    }
+
+    private boolean filesystemUsesBackslashSeparator() {
+        // TODO note: this not 100% accurate because the filesystem that the repository is on may be different than the
+        //            default filesystem
+        var pathSeparator = FileSystems.getDefault().getSeparator().charAt(0);
+        return pathSeparator == '\\';
+    }
+
 }

ocfl-java-itest/src/test/java/edu/wisc/library/ocfl/itest/OcflITest.java

@@ -39,6 +39,7 @@
 import edu.wisc.library.ocfl.itest.ext.TestLayoutExtensionConfig;
 import edu.wisc.library.ocfl.test.OcflAsserts;
 import edu.wisc.library.ocfl.test.TestHelper;
+import org.apache.commons.io.IOUtils;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -47,6 +48,7 @@
 import java.io.BufferedInputStream;
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
+import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
@@ -88,6 +90,7 @@
 import static org.junit.jupiter.api.Assertions.assertSame;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
 
 public abstract class OcflITest {
 
@@ -860,6 +863,21 @@ public void failGetObjectWhenInvalidDigestAlgorithmUsed() {
         }).getMessage(), containsString("digestAlgorithm must be sha512 or sha256"));
     }
 
+    @Test
+    public void failReadingObjectWhenHasMaliciousPath() {
+        var repoName = "malicious-content-paths";
+        var repoDir = sourceRepoPath(repoName);
+        var repo = existingRepo(repoName, repoDir);
+
+        assertThrows(PathConstraintException.class, () -> {
+            try (var stream = repo.getObject(ObjectVersionId.head("urn:example:bad"))
+                    .getFile("file.txt").getStream()) {
+                IOUtils.toString(stream, StandardCharsets.UTF_8);
+                fail("Should not have read file");
+            }
+        });
+    }
+
     @Test
     public void putObjectWithDuplicateFiles() {
         var repoName = "repo8";

ocfl-java-itest/src/test/resources/sources/repos/malicious-content-paths/0=ocfl_1.0

@@ -0,0 +1 @@
+ocfl_1.0

ocfl-java-itest/src/test/resources/sources/repos/malicious-content-paths/692/287/678/692287678fe1a8b750e230228b0e7bb145eca11fa7751a8ef336bd68962e1a5e/0=ocfl_object_1.0

@@ -0,0 +1 @@
+ocfl_object_1.0

ocfl-java-itest/src/test/resources/sources/repos/malicious-content-paths/692/287/678/692287678fe1a8b750e230228b0e7bb145eca11fa7751a8ef336bd68962e1a5e/inventory.json

@@ -0,0 +1,26 @@
+{
+  "id": "urn:example:bad",
+  "type": "https://ocfl.io/1.0/spec/#inventory",
+  "digestAlgorithm": "sha512",
+  "head": "v1",
+  "contentDirectory": "content",
+  "manifest": {
+    "0e3e75234abc68f4378a86b3f4b32a198ba301845b0cd6e50106e874345700cc6663a86c1ea125dc5e92be17c98f9a0f85ca9d5f595db2012f7cc3571945c123": [
+      "v1/content/../../../../../../0=ocfl_1.0"
+    ]
+  },
+  "versions": {
+    "v1": {
+      "created": "2022-02-11T13:36:06.106847171-06:00",
+      "state": {
+        "0e3e75234abc68f4378a86b3f4b32a198ba301845b0cd6e50106e874345700cc6663a86c1ea125dc5e92be17c98f9a0f85ca9d5f595db2012f7cc3571945c123": [
+          "file.txt"
+        ]
+      },
+      "user": {
+        "name": "Peter Winckles",
+        "address": "mailto:pwinckles@pm.me"
+      }
+    }
+  }
+}

ocfl-java-itest/src/test/resources/sources/repos/malicious-content-paths/692/287/678/692287678fe1a8b750e230228b0e7bb145eca11fa7751a8ef336bd68962e1a5e/inventory.json.sha512

@@ -0,0 +1 @@
+d7cbc89e4f8f79d363d1543cb68c893dc2ad7d905f88e96e5694eeaba41d878496f7234c0eacf8ad437dfc8bfb583e16bc2850657209eb0ba30ff7bf3ca5b74a  inventory.json

ocfl-java-itest/src/test/resources/sources/repos/malicious-content-paths/692/287/678/692287678fe1a8b750e230228b0e7bb145eca11fa7751a8ef336bd68962e1a5e/v1/content/file.txt

@@ -0,0 +1 @@
+test

ocfl-java-itest/src/test/resources/sources/repos/malicious-content-paths/692/287/678/692287678fe1a8b750e230228b0e7bb145eca11fa7751a8ef336bd68962e1a5e/v1/inventory.json

@@ -0,0 +1,26 @@
+{
+  "id": "urn:example:bad",
+  "type": "https://ocfl.io/1.0/spec/#inventory",
+  "digestAlgorithm": "sha512",
+  "head": "v1",
+  "contentDirectory": "content",
+  "manifest": {
+    "0e3e75234abc68f4378a86b3f4b32a198ba301845b0cd6e50106e874345700cc6663a86c1ea125dc5e92be17c98f9a0f85ca9d5f595db2012f7cc3571945c123": [
+      "v1/content/../../../../../../0=ocfl_1.0"
+    ]
+  },
+  "versions": {
+    "v1": {
+      "created": "2022-02-11T13:36:06.106847171-06:00",
+      "state": {
+        "0e3e75234abc68f4378a86b3f4b32a198ba301845b0cd6e50106e874345700cc6663a86c1ea125dc5e92be17c98f9a0f85ca9d5f595db2012f7cc3571945c123": [
+          "file.txt"
+        ]
+      },
+      "user": {
+        "name": "Peter Winckles",
+        "address": "mailto:pwinckles@pm.me"
+      }
+    }
+  }
+}

ocfl-java-itest/src/test/resources/sources/repos/malicious-content-paths/692/287/678/692287678fe1a8b750e230228b0e7bb145eca11fa7751a8ef336bd68962e1a5e/v1/inventory.json.sha512

@@ -0,0 +1 @@
+d7cbc89e4f8f79d363d1543cb68c893dc2ad7d905f88e96e5694eeaba41d878496f7234c0eacf8ad437dfc8bfb583e16bc2850657209eb0ba30ff7bf3ca5b74a  inventory.json

ocfl-java-itest/src/test/resources/sources/repos/malicious-content-paths/extensions/0004-hashed-n-tuple-storage-layout/config.json

@@ -0,0 +1,7 @@
+{
+  "extensionName": "0004-hashed-n-tuple-storage-layout",
+  "digestAlgorithm": "sha256",
+  "tupleSize": 3,
+  "numberOfTuples": 3,
+  "shortObjectRoot": false
+}

ocfl-java-itest/src/test/resources/sources/repos/malicious-content-paths/ocfl_layout.json

@@ -0,0 +1,4 @@
+{
+  "extension": "0004-hashed-n-tuple-storage-layout",
+  "description": "See specification document 0004-hashed-n-tuple-storage-layout.md"
+}