From 5d256d3d452b82d8b60ae6c284f851b35ca9f1c6 Mon Sep 17 00:00:00 2001 From: Matteo Merli Date: Mon, 3 Jun 2024 16:50:56 -0700 Subject: [PATCH 1/2] [improve] Validate user paths in Functions utils --- .../functions/utils/FunctionConfigUtils.java | 14 ++++++++++++-- .../filesystem/FileSystemPackagesStorage.java | 6 +++++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/pulsar-functions/utils/src/main/java/org/apache/pulsar/functions/utils/FunctionConfigUtils.java b/pulsar-functions/utils/src/main/java/org/apache/pulsar/functions/utils/FunctionConfigUtils.java index ee59317daf755..9dc9d5428eda3 100644 --- a/pulsar-functions/utils/src/main/java/org/apache/pulsar/functions/utils/FunctionConfigUtils.java +++ b/pulsar-functions/utils/src/main/java/org/apache/pulsar/functions/utils/FunctionConfigUtils.java @@ -853,14 +853,24 @@ public static void doCommonChecks(FunctionConfig functionConfig) { if (!isEmpty(functionConfig.getPy()) && !org.apache.pulsar.common.functions.Utils .isFunctionPackageUrlSupported(functionConfig.getPy()) && functionConfig.getPy().startsWith(BUILTIN)) { - if (!new File(functionConfig.getPy()).exists()) { + String filename = functionConfig.getPy(); + if (filename.contains("..")) { + throw new IllegalArgumentException("Invalid filename: " + filename); + } + + if (!new File(filename).exists()) { throw new IllegalArgumentException("The supplied python file does not exist"); } } if (!isEmpty(functionConfig.getGo()) && !org.apache.pulsar.common.functions.Utils .isFunctionPackageUrlSupported(functionConfig.getGo()) && functionConfig.getGo().startsWith(BUILTIN)) { - if (!new File(functionConfig.getGo()).exists()) { + String filename = functionConfig.getGo(); + if (filename.contains("..")) { + throw new IllegalArgumentException("Invalid filename: " + filename); + } + + if (!new File(filename).exists()) { throw new IllegalArgumentException("The supplied go file does not exist"); } } diff --git a/pulsar-package-management/filesystem-storage/src/main/java/org/apache/pulsar/packages/management/storage/filesystem/FileSystemPackagesStorage.java b/pulsar-package-management/filesystem-storage/src/main/java/org/apache/pulsar/packages/management/storage/filesystem/FileSystemPackagesStorage.java index 47d825ea928f4..9513b3617e254 100644 --- a/pulsar-package-management/filesystem-storage/src/main/java/org/apache/pulsar/packages/management/storage/filesystem/FileSystemPackagesStorage.java +++ b/pulsar-package-management/filesystem-storage/src/main/java/org/apache/pulsar/packages/management/storage/filesystem/FileSystemPackagesStorage.java @@ -58,7 +58,11 @@ public class FileSystemPackagesStorage implements PackagesStorage { } } - private File getPath(String path) { + private File getPath(String path) throws IOException { + if (path.contains("..")) { + throw new IOException("Invalid path: " + path); + } + File f = Paths.get(storagePath.toString(), path).toFile(); if (!f.getParentFile().exists()) { if (!f.getParentFile().mkdirs()) { From 6164a8a7aa74875b5daa3ac5c6376d68206d3f20 Mon Sep 17 00:00:00 2001 From: Matteo Merli Date: Mon, 3 Jun 2024 16:59:10 -0700 Subject: [PATCH 2/2] added try/catch --- .../pulsar/broker/web/ExceptionHandler.java | 2 ++ .../filesystem/FileSystemPackagesStorage.java | 36 ++++++++++++------- 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/web/ExceptionHandler.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/web/ExceptionHandler.java index b11ec3a8a98db..205e02ed75a2e 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/web/ExceptionHandler.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/web/ExceptionHandler.java @@ -24,6 +24,7 @@ import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletResponse; import javax.ws.rs.core.Response; +import lombok.extern.slf4j.Slf4j; import org.apache.pulsar.common.intercept.InterceptException; import org.apache.pulsar.common.policies.data.ErrorData; import org.apache.pulsar.common.util.ObjectMapperFactory; @@ -36,6 +37,7 @@ /** * Exception handler for handle exception. */ +@Slf4j public class ExceptionHandler { public void handle(ServletResponse response, Exception ex) throws IOException { diff --git a/pulsar-package-management/filesystem-storage/src/main/java/org/apache/pulsar/packages/management/storage/filesystem/FileSystemPackagesStorage.java b/pulsar-package-management/filesystem-storage/src/main/java/org/apache/pulsar/packages/management/storage/filesystem/FileSystemPackagesStorage.java index 9513b3617e254..2bb43bb207203 100644 --- a/pulsar-package-management/filesystem-storage/src/main/java/org/apache/pulsar/packages/management/storage/filesystem/FileSystemPackagesStorage.java +++ b/pulsar-package-management/filesystem-storage/src/main/java/org/apache/pulsar/packages/management/storage/filesystem/FileSystemPackagesStorage.java @@ -123,28 +123,40 @@ public CompletableFuture readAsync(String path, OutputStream outputStream) @Override public CompletableFuture deleteAsync(String path) { - if (getPath(path).delete()) { - return CompletableFuture.completedFuture(null); - } else { - CompletableFuture f = new CompletableFuture<>(); - f.completeExceptionally(new IOException("Failed to delete file at " + path)); - return f; + try { + if (getPath(path).delete()) { + return CompletableFuture.completedFuture(null); + } else { + CompletableFuture f = new CompletableFuture<>(); + f.completeExceptionally(new IOException("Failed to delete file at " + path)); + return f; + } + } catch (IOException e) { + return CompletableFuture.failedFuture(e); } } @Override public CompletableFuture> listAsync(String path) { - String[] files = getPath(path).list(); - if (files == null) { - return CompletableFuture.completedFuture(Collections.emptyList()); - } else { - return CompletableFuture.completedFuture(Arrays.asList(files)); + try { + String[] files = getPath(path).list(); + if (files == null) { + return CompletableFuture.completedFuture(Collections.emptyList()); + } else { + return CompletableFuture.completedFuture(Arrays.asList(files)); + } + } catch (IOException e) { + return CompletableFuture.failedFuture(e); } } @Override public CompletableFuture existAsync(String path) { - return CompletableFuture.completedFuture(getPath(path).exists()); + try { + return CompletableFuture.completedFuture(getPath(path).exists()); + } catch (IOException e) { + return CompletableFuture.failedFuture(e); + } } @Override