Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 38 additions & 8 deletions util/src/main/java/io/kubernetes/client/Copy.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.io.OutputStream;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Objects;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
Expand Down Expand Up @@ -241,8 +242,7 @@ private void createFiles(
for (TreeNode childNode : node.getChildren()) {
if (!childNode.isDir) {
// Create file which is under 'node'
String filePath = genericPathBuilder(destination.toString(), childNode.name);
File f = new File(filePath);
File f = safeResolveWithinBase(destination, childNode.name).toFile();
if (!f.createNewFile()) {
throw new IOException("Failed to create file: " + f);
}
Expand All @@ -254,8 +254,7 @@ private void createFiles(
}
} else {
String newSrcPath = genericPathBuilder(srcPath, childNode.name);
// TODO: Change this once the method genericPathBuilder is changed to varargs
Path newDestination = Paths.get(destination.toString(), childNode.name);
Path newDestination = safeResolveWithinBase(destination, childNode.name);
createFiles(childNode, namespace, pod, container, newSrcPath, newDestination);
}
}
Expand All @@ -276,8 +275,7 @@ private void createDirectory(TreeNode node, Path destination) throws IOException
}
for (TreeNode childNode : node.getChildren()) {
if (childNode.isDir) {
String path = genericPathBuilder(destination.toString(), childNode.name);
Path newPath = Paths.get(path);
Path newPath = safeResolveWithinBase(destination, childNode.name);
createDirectory(childNode, newPath);
}
}
Expand All @@ -304,22 +302,54 @@ private void createDirectoryTree(
int len = line.length();
// line = stripFileSeparators(line);
if (line.charAt(line.length() - 1) == '/') {
String safeName = sanitizeRelativeEntryName(line.substring(0, len - 1));
TreeNode subDirTree =
new TreeNode(
true,
line.substring(0, len - 1),
safeName,
false); // Stripping off '/' in the end of directory
String path = genericPathBuilder(srcPath, subDirTree.name);
createDirectoryTree(subDirTree, namespace, pod, container, path);
node.getChildren().add(subDirTree);
} else {
node.getChildren().add(new TreeNode(false, line, false));
node.getChildren().add(new TreeNode(false, sanitizeRelativeEntryName(line), false));
}
}
}
}
}

private Path safeResolveWithinBase(Path base, String entryName) throws IOException {
Path normalizedBase = base.toAbsolutePath().normalize();
Path resolved = normalizedBase.resolve(entryName).normalize();
if (!resolved.startsWith(normalizedBase)) {
throw new IOException("Invalid path entry: " + entryName);
}
return resolved;
}

private String sanitizeRelativeEntryName(String entryName) throws IOException {
String sanitized = Objects.requireNonNull(entryName, "entryName").trim();
if (sanitized.isEmpty()) {
throw new IOException("Invalid empty path entry");
}

// ls -F output should be a single relative path segment. Reject path separators and traversal.
if (sanitized.contains("/") || sanitized.contains("\\")) {
throw new IOException("Invalid path entry: " + entryName);
}

String normalized = FilenameUtils.normalize(sanitized, true);
if (normalized == null || normalized.startsWith("../") || normalized.startsWith("/")) {
throw new IOException("Invalid path entry: " + entryName);
}

if (normalized.endsWith("/")) {
normalized = normalized.substring(0, normalized.length() - 1);
}
return normalized;
}

/*
Generic method to create path.
TODO: Change this to varargs
Expand Down
123 changes: 123 additions & 0 deletions util/src/test/java/io/kubernetes/client/CopyTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,25 @@
import io.kubernetes.client.openapi.models.V1Pod;
import io.kubernetes.client.util.ClientBuilder;
import io.kubernetes.client.util.exception.CopyNotSupportedException;
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.HashMap;
import java.util.Map;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.junit.jupiter.api.io.TempDir;

import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

/** Tests for the Copy helper class */
class CopyTest {
private String namespace;
Expand All @@ -45,6 +54,71 @@ class CopyTest {

private ApiClient client;

private static class MockProcess extends Process {
private final InputStream inputStream;

MockProcess(String stdout) {
this.inputStream = new ByteArrayInputStream(stdout.getBytes());
}

@Override
public OutputStream getOutputStream() {
return OutputStream.nullOutputStream();
}

@Override
public InputStream getInputStream() {
return inputStream;
}

@Override
public InputStream getErrorStream() {
return InputStream.nullInputStream();
}

@Override
public int waitFor() {
return 0;
}

@Override
public int exitValue() {
return 0;
}

@Override
public void destroy() {
// no-op for tests
}
}

private static class MockCopy extends Copy {
private final Map<String, String> listingsByPath;

MockCopy(Map<String, String> listingsByPath) {
this.listingsByPath = listingsByPath;
}

@Override
public Process exec(
String namespace,
String name,
String[] command,
String container,
boolean stdin,
boolean tty) {
String cmd = command[2];
String srcPath = cmd.substring("ls -F ".length());
return new MockProcess(listingsByPath.getOrDefault(srcPath, ""));
}

@Override
public InputStream copyFileFromPod(String namespace, String pod, String srcPath)
throws ApiException, IOException {
return new ByteArrayInputStream("test-data".getBytes());
}
}

@RegisterExtension
static WireMockExtension apiServer =
WireMockExtension.newInstance().options(wireMockConfig().dynamicPort()).build();
Expand Down Expand Up @@ -225,4 +299,53 @@ public void run() {
.withQueryParam("command", equalTo("-c"))
.withQueryParam("command", equalTo("tar --version")));
}

@Test
void rejectsPathTraversalInNonTarCopy(@TempDir Path tempDir) throws Exception {
Path sourceRoot = Paths.get("/src");
Path destinationRoot = tempDir.resolve("dest");
Path escapedFile = tempDir.resolve("escape.txt");

Map<String, String> listingsByPath = new HashMap<>();
listingsByPath.put(sourceRoot.toString(), "../escape.txt\n");

Copy copy = new MockCopy(listingsByPath);

assertThrows(
IOException.class,
() ->
copy.copyDirectoryFromPod(
namespace,
podName,
null,
sourceRoot.toString(),
destinationRoot,
false));

assertFalse(Files.exists(escapedFile));
}

@Test
void copiesSafeEntriesInNonTarMode(@TempDir Path tempDir) throws Exception {
Path sourceRoot = Paths.get("/src");
Path destinationRoot = tempDir.resolve("dest");
Files.createDirectories(destinationRoot);

Map<String, String> listingsByPath = new HashMap<>();
listingsByPath.put(sourceRoot.toString(), "safe.txt\n");

Copy copy = new MockCopy(listingsByPath);

copy.copyDirectoryFromPod(
namespace,
podName,
null,
sourceRoot.toString(),
destinationRoot,
false);

Path copied = destinationRoot.resolve("safe.txt");
assertTrue(Files.exists(copied));
assertEquals("test-data", Files.readString(copied));
}
}
Loading