diff --git a/xtraplatform-blobs/src/main/java/de/ii/xtraplatform/blobs/app/BlobCacheImpl.java b/xtraplatform-blobs/src/main/java/de/ii/xtraplatform/blobs/app/BlobCacheImpl.java index 42c4d503..70ca85d9 100644 --- a/xtraplatform-blobs/src/main/java/de/ii/xtraplatform/blobs/app/BlobCacheImpl.java +++ b/xtraplatform-blobs/src/main/java/de/ii/xtraplatform/blobs/app/BlobCacheImpl.java @@ -55,7 +55,21 @@ public Path put(Path path, String eTag, InputStream content) throws IOException private Path getCachePath(Path path, String eTag) { String fileDir = path.getFileName().toString().replaceAll("\\.", "_"); + // The eTag comes from the (remote) source and becomes a path segment; reduce it to safe + // filename + // characters so it cannot contain separators or `..`. + String safeETag = eTag.replaceAll("[^A-Za-z0-9._-]", "_"); - return tmpDirectory.resolve(path.getParent()).resolve(fileDir).resolve(eTag); + Path cachePath = + tmpDirectory.resolve(path.getParent()).resolve(fileDir).resolve(safeETag).normalize(); + + // Defense-in-depth: `path` may carry `..` segments; ensure the cache entry stays inside the + // cache directory. + if (!cachePath.startsWith(tmpDirectory.normalize())) { + throw new IllegalArgumentException( + "The cache path for '" + path + "' would escape the cache directory."); + } + + return cachePath; } } diff --git a/xtraplatform-blobs/src/main/java/de/ii/xtraplatform/blobs/infra/BlobSourceFs.java b/xtraplatform-blobs/src/main/java/de/ii/xtraplatform/blobs/infra/BlobSourceFs.java index d27e6771..c4daa5d2 100644 --- a/xtraplatform-blobs/src/main/java/de/ii/xtraplatform/blobs/infra/BlobSourceFs.java +++ b/xtraplatform-blobs/src/main/java/de/ii/xtraplatform/blobs/infra/BlobSourceFs.java @@ -213,8 +213,16 @@ private Path full(Path path) { return Objects.isNull(prefix) ? root.resolve(path) : root.resolve(prefix.relativize(path)); } + // The resolved path must stay inside the store root. This rejects keys containing `..` segments + // or an absolute path that would otherwise escape the root (e.g. via full() -> root.resolve(..)). + // Every read/write/delete method gates on canHandle (directly or via has), so a traversal key is + // treated as "not handled" — a no-op / not-found — rather than escaping the store. + private boolean isContained(Path path) { + return full(path).normalize().startsWith(root.normalize()); + } + @Override public boolean canHandle(Path path) { - return Objects.isNull(prefix) || path.startsWith(prefix); + return (Objects.isNull(prefix) || path.startsWith(prefix)) && isContained(path); } } diff --git a/xtraplatform-blobs/src/test/groovy/de/ii/xtraplatform/blobs/infra/BlobSourceFsContainmentSpec.groovy b/xtraplatform-blobs/src/test/groovy/de/ii/xtraplatform/blobs/infra/BlobSourceFsContainmentSpec.groovy new file mode 100644 index 00000000..2c9f5380 --- /dev/null +++ b/xtraplatform-blobs/src/test/groovy/de/ii/xtraplatform/blobs/infra/BlobSourceFsContainmentSpec.groovy @@ -0,0 +1,79 @@ +/* + * Copyright 2025 interactive instruments GmbH + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ +package de.ii.xtraplatform.blobs.infra + +import spock.lang.Specification + +import java.nio.file.Files +import java.nio.file.Path + +class BlobSourceFsContainmentSpec extends Specification { + + Path root + Path secret + + def setup() { + Path base = Files.createTempDirectory("blobstore") + root = Files.createDirectory(base.resolve("store")) + Files.writeString(root.resolve("inside.txt"), "inside") + // a file outside the store root + secret = base.resolve("secret.txt") + Files.writeString(secret, "secret") + } + + def "legitimate keys (including ones that normalize back inside) are handled"() { + given: + BlobSourceFs source = new BlobSourceFs(root) + + expect: + source.canHandle(Path.of("inside.txt")) + source.has(Path.of("inside.txt")) + source.canHandle(Path.of("sub/../inside.txt")) + } + + def "traversal and absolute keys are rejected"() { + given: + BlobSourceFs source = new BlobSourceFs(root) + + expect: + !source.canHandle(key) + !source.has(key) + source.content(key).isEmpty() + source.size(key) == -1 + + where: + key << [ + Path.of("../secret.txt"), + Path.of("a/../../secret.txt"), + Path.of("/etc/passwd"), + ] + } + + def "a traversal delete does not remove files outside the root"() { + given: + BlobSourceFs source = new BlobSourceFs(root) + + when: + source.delete(Path.of("../secret.txt")) + + then: + Files.exists(secret) + } + + def "a traversal put does not write files outside the root"() { + given: + BlobSourceFs source = new BlobSourceFs(root) + Path target = root.resolveSibling("evil.txt") + + when: + source.put(Path.of("../evil.txt"), new ByteArrayInputStream("x".getBytes("UTF-8"))) + + then: + !Files.exists(target) + } +}