From cfb435604c24a1e301fa229bc5d59f55e7398d53 Mon Sep 17 00:00:00 2001 From: Clemens Portele Date: Thu, 2 Jul 2026 14:10:20 +0200 Subject: [PATCH] fix path traversal in the file blob store MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BlobSourceFs resolved a key against the store root without normalizing or checking containment, so a key with `..` segments or an absolute path escaped the root — every read/write/delete method (has/content/get/size/lastModified/ walk/put/delete) was affected. This is the root cause behind the traversal reachable through request-derived keys in the 3D-tiles and resources endpoints. Enforce the invariant in canHandle: the normalized resolved path must start with the normalized root. Every method already gates on canHandle (directly or via has), so a traversal key is now treated as "not handled" — a no-op or not-found — instead of escaping the store. Also harden BlobCacheImpl.getCachePath, which built a cache path from the blob key and the (source-provided) ETag: reduce the ETag to safe filename characters and reject a cache path that would resolve outside the cache directory. Adds BlobSourceFsContainmentSpec. --- .../xtraplatform/blobs/app/BlobCacheImpl.java | 16 +++- .../blobs/infra/BlobSourceFs.java | 10 ++- .../infra/BlobSourceFsContainmentSpec.groovy | 79 +++++++++++++++++++ 3 files changed, 103 insertions(+), 2 deletions(-) create mode 100644 xtraplatform-blobs/src/test/groovy/de/ii/xtraplatform/blobs/infra/BlobSourceFsContainmentSpec.groovy 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) + } +}