Fix possible race condition during module and resource reading (#1426)

This commit is contained in:
Islon Scherer
2026-02-11 11:18:11 +01:00
committed by GitHub
parent 817e433a7f
commit 60f628eb36
5 changed files with 93 additions and 13 deletions

View File

@@ -1,5 +1,5 @@
/*
* Copyright © 2024 Apple Inc. and the Pkl project authors. All rights reserved.
* Copyright © 2024-2026 Apple Inc. and the Pkl project authors. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -15,7 +15,10 @@
*/
package org.pkl.core;
import java.io.IOException;
import java.net.URI;
import java.nio.file.Path;
import org.pkl.core.util.Nullable;
/**
* Enforces a security model during {@link Evaluator evaluation}.
@@ -40,4 +43,20 @@ public interface SecurityManager {
* to access the given URI.
*/
void checkResolveResource(URI resource) throws SecurityManagerException;
/**
* Resolves the given {@code file:} URI to a secure, symlink-free path that has been verified to
* be within the root directory (if one is configured). The returned path can be opened with
* {@link java.nio.file.LinkOption#NOFOLLOW_LINKS}.
*
* <p>Returns {@code null} for non-{@code file:} URIs or if no root directory is configured.
*
* @param uri the URI to resolve
* @return the resolved, symlink-free path under root directory, or {@code null}
* @throws SecurityManagerException if the resolved path is not within the root directory
* @throws IOException if the path cannot be resolved
*/
default @Nullable Path resolveSecurePath(URI uri) throws SecurityManagerException, IOException {
return null;
}
}

View File

@@ -1,5 +1,5 @@
/*
* Copyright © 2024-2025 Apple Inc. and the Pkl project authors. All rights reserved.
* Copyright © 2024-2026 Apple Inc. and the Pkl project authors. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -171,6 +171,19 @@ public final class SecurityManagers {
}
}
@Override
public @Nullable Path resolveSecurePath(URI uri) throws SecurityManagerException, IOException {
if (rootDir == null || !uri.isAbsolute() || !uri.getScheme().equals("file")) {
return null;
}
var path = Path.of(uri);
var realPath = path.toRealPath();
if (!realPath.startsWith(rootDir)) {
throw new SecurityManagerException(ErrorMessages.create("modulePastRootDir", uri, rootDir));
}
return realPath;
}
private @Nullable Path normalizePath(@Nullable Path path) {
if (path == null) {
return null;

View File

@@ -1,5 +1,5 @@
/*
* Copyright © 2024-2025 Apple Inc. and the Pkl project authors. All rights reserved.
* Copyright © 2024-2026 Apple Inc. and the Pkl project authors. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -345,10 +345,18 @@ public final class ModuleKeys {
if (java.io.File.separatorChar == '\\' && uriPath != null && uriPath.contains("\\")) {
throw new FileNotFoundException();
}
var realPath = IoUtils.pathOf(uri).toRealPath();
// Use resolveSecurePath to atomically resolve symlinks and verify under rootDir.
// The returned path is symlink-free, so it can be opened with NOFOLLOW_LINKS.
var securePath = securityManager.resolveSecurePath(uri);
Path realPath;
if (securePath != null) {
realPath = securePath;
} else {
realPath = IoUtils.pathOf(uri).toRealPath();
}
var resolvedUri = realPath.toUri();
securityManager.checkResolveModule(resolvedUri);
return ResolvedModuleKeys.file(this, resolvedUri, realPath);
return ResolvedModuleKeys.file(this, resolvedUri, realPath, securePath != null);
}
@Override
@@ -413,8 +421,14 @@ public final class ModuleKeys {
throws IOException, SecurityManagerException {
securityManager.checkResolveModule(uri);
var path = resolver.resolve(uri).toRealPath();
return ResolvedModuleKeys.file(this, path.toUri(), path);
var securePath = securityManager.resolveSecurePath(uri);
Path path;
if (securePath != null) {
path = securePath;
} else {
path = resolver.resolve(uri).toRealPath();
}
return ResolvedModuleKeys.file(this, path.toUri(), path, securePath != null);
}
}

View File

@@ -1,5 +1,5 @@
/*
* Copyright © 2024 Apple Inc. and the Pkl project authors. All rights reserved.
* Copyright © 2024-2026 Apple Inc. and the Pkl project authors. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -15,13 +15,13 @@
*/
package org.pkl.core.module;
import java.io.File;
import java.io.IOException;
import java.net.URI;
import java.net.URL;
import java.nio.charset.StandardCharsets;
import java.nio.file.AccessDeniedException;
import java.nio.file.Files;
import java.nio.file.LinkOption;
import java.nio.file.Path;
import org.pkl.core.util.IoUtils;
@@ -29,12 +29,22 @@ import org.pkl.core.util.IoUtils;
public final class ResolvedModuleKeys {
private ResolvedModuleKeys() {}
/**
* Creates a resolved module key backed by the given file path. The resulting module will be
* loaded from that file path and cached using the given URI as cache key.
*
* @param nofollow if true, the file will be opened with {@link LinkOption#NOFOLLOW_LINKS}.
*/
public static ResolvedModuleKey file(ModuleKey original, URI uri, Path path, boolean nofollow) {
return new FileKey(original, uri, path, nofollow);
}
/**
* Creates a resolved module key backed by the given file path. The resulting module will be
* loaded from that file path and cached using the given URI as cache key.
*/
public static ResolvedModuleKey file(ModuleKey original, URI uri, Path path) {
return new File(original, uri, path);
return new FileKey(original, uri, path, false);
}
/**
@@ -62,15 +72,17 @@ public final class ResolvedModuleKeys {
return new Delegated(delegate, original);
}
private static class File implements ResolvedModuleKey {
private static class FileKey implements ResolvedModuleKey {
final ModuleKey original;
final URI uri;
final Path path;
final boolean nofollow;
File(ModuleKey original, URI uri, Path path) {
FileKey(ModuleKey original, URI uri, Path path, boolean nofollow) {
this.original = original;
this.uri = uri;
this.path = path;
this.nofollow = nofollow;
}
@Override
@@ -86,6 +98,11 @@ public final class ResolvedModuleKeys {
@Override
public String loadSource() throws IOException {
try {
if (nofollow) {
try (var in = Files.newInputStream(path, LinkOption.NOFOLLOW_LINKS)) {
return new String(in.readAllBytes(), StandardCharsets.UTF_8);
}
}
return Files.readString(path, StandardCharsets.UTF_8);
} catch (AccessDeniedException e) {
// Windows throws `AccessDeniedException` when reading directories.

View File

@@ -1,5 +1,5 @@
/*
* Copyright © 2024-2025 Apple Inc. and the Pkl project authors. All rights reserved.
* Copyright © 2024-2026 Apple Inc. and the Pkl project authors. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -22,6 +22,8 @@ import java.net.URISyntaxException;
import java.net.http.HttpRequest;
import java.net.http.HttpResponse.BodyHandlers;
import java.nio.file.Files;
import java.nio.file.LinkOption;
import java.nio.file.NoSuchFileException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
@@ -257,6 +259,21 @@ public final class ResourceReaders {
@Override
public Optional<Object> read(URI uri) throws IOException, URISyntaxException {
IoUtils.validateFileUri(uri);
// Use resolveSecurePath to get a symlink-free path verified under rootDir.
var securityManager = VmContext.get(null).getSecurityManager();
try {
var securePath = securityManager.resolveSecurePath(uri);
if (securePath != null) {
try (var in = Files.newInputStream(securePath, LinkOption.NOFOLLOW_LINKS)) {
var content = in.readAllBytes();
return Optional.of(new Resource(uri, content));
} catch (NoSuchFileException e) {
return Optional.empty();
}
}
} catch (SecurityManagerException e) {
throw new IOException(e);
}
return super.read(uri);
}