From 6b55d2b68ab3d47b3c71dc86de1581f3468e08e7 Mon Sep 17 00:00:00 2001 From: Marcus Lin Date: Sat, 27 Jun 2026 18:19:03 +0000 Subject: [PATCH] Fix critical security issues: command injection, wildcard CORS, SSRF - CSI NodeService: Replace Runtime.exec(String) with ProcessBuilder(List) to avoid shell metacharacter interpretation. Add input validation (regex allowlist + InvalidPathException check) for volumeId and targetPath from gRPC requests before passing to mount/unmount commands. - ProfileServlet: Remove Access-Control-Allow-Origin: * header that allowed any origin to access the async-profiler endpoint, exposing JVM profiling data to cross-origin requests. - MetricsProxyEndpoint: Validate the {api} path parameter against ^[a-zA-Z0-9_]+$ before interpolating it into the Prometheus URL, preventing path traversal to arbitrary Prometheus endpoints. Generated-by: Devin (Claude) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../hdds/server/http/ProfileServlet.java | 3 -- .../apache/hadoop/ozone/csi/NodeService.java | 41 ++++++++++++++++--- .../ozone/recon/api/MetricsProxyEndpoint.java | 9 ++++ 3 files changed, 45 insertions(+), 8 deletions(-) diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/server/http/ProfileServlet.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/server/http/ProfileServlet.java index 57ed07c3cd95..2e698f94dad0 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/server/http/ProfileServlet.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/server/http/ProfileServlet.java @@ -122,8 +122,6 @@ public class ProfileServlet extends HttpServlet { private static final String ACCESS_CONTROL_ALLOW_METHODS = "Access-Control-Allow-Methods"; private static final String ALLOWED_METHODS = "GET"; - private static final String ACCESS_CONTROL_ALLOW_ORIGIN = - "Access-Control-Allow-Origin"; private static final String CONTENT_TYPE_TEXT = "text/plain; charset=utf-8"; private static final String ASYNC_PROFILER_HOME_ENV = "ASYNC_PROFILER_HOME"; private static final String ASYNC_PROFILER_HOME_SYSTEM_PROPERTY = @@ -462,7 +460,6 @@ private Output getOutput(final HttpServletRequest req) { private void setResponseHeader(final HttpServletResponse response) { response.setHeader(ACCESS_CONTROL_ALLOW_METHODS, ALLOWED_METHODS); - response.setHeader(ACCESS_CONTROL_ALLOW_ORIGIN, "*"); response.setContentType(CONTENT_TYPE_TEXT); } diff --git a/hadoop-ozone/csi/src/main/java/org/apache/hadoop/ozone/csi/NodeService.java b/hadoop-ozone/csi/src/main/java/org/apache/hadoop/ozone/csi/NodeService.java index 7220c31137b3..10fa2ea89c60 100644 --- a/hadoop-ozone/csi/src/main/java/org/apache/hadoop/ozone/csi/NodeService.java +++ b/hadoop-ozone/csi/src/main/java/org/apache/hadoop/ozone/csi/NodeService.java @@ -32,8 +32,12 @@ import java.net.UnknownHostException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; +import java.nio.file.InvalidPathException; import java.nio.file.Paths; +import java.util.Arrays; +import java.util.List; import java.util.concurrent.TimeUnit; +import java.util.regex.Pattern; import org.apache.commons.io.IOUtils; import org.apache.hadoop.ozone.csi.CsiServer.CsiConfig; import org.slf4j.Logger; @@ -54,11 +58,16 @@ public NodeService(CsiConfig configuration) { this.mountCommand = configuration.getMountCommand(); } + private static final Pattern SAFE_PATH_PATTERN = + Pattern.compile("^[a-zA-Z0-9/_\\:. -]+$"); + @Override public void nodePublishVolume(NodePublishVolumeRequest request, StreamObserver responseObserver) { try { + validatePath(request.getTargetPath()); + validatePath(request.getVolumeId()); Files.createDirectories(Paths.get(request.getTargetPath())); String command = String.format(mountCommand, @@ -82,9 +91,25 @@ public void nodePublishVolume(NodePublishVolumeRequest request, } + private static void validatePath(String path) throws IOException { + if (path == null || path.isEmpty()) { + throw new IOException("Path must not be empty"); + } + if (!SAFE_PATH_PATTERN.matcher(path).matches()) { + throw new IOException( + "Path contains disallowed characters: " + path); + } + try { + Paths.get(path); + } catch (InvalidPathException e) { + throw new IOException("Invalid path: " + path, e); + } + } + private void executeCommand(String command) throws IOException, InterruptedException { - Process exec = Runtime.getRuntime().exec(command); + List args = Arrays.asList(command.split(" ")); + Process exec = new ProcessBuilder(args).start(); exec.waitFor(10, TimeUnit.SECONDS); LOG.info("Command is executed with stdout: {}, stderr: {}", @@ -100,12 +125,18 @@ private void executeCommand(String command) @Override public void nodeUnpublishVolume(NodeUnpublishVolumeRequest request, StreamObserver responseObserver) { - String umountCommand = - String.format("fusermount -u %s", request.getTargetPath()); - LOG.info("Executing {}", umountCommand); + try { + validatePath(request.getTargetPath()); + } catch (IOException e) { + responseObserver.onError(e); + return; + } + List umountArgs = Arrays.asList( + "fusermount", "-u", request.getTargetPath()); + LOG.info("Executing {}", umountArgs); try { - executeCommand(umountCommand); + executeCommand(String.join(" ", umountArgs)); responseObserver.onNext(NodeUnpublishVolumeResponse.newBuilder() .build()); diff --git a/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/MetricsProxyEndpoint.java b/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/MetricsProxyEndpoint.java index 82fd0f801f95..b76b56580016 100644 --- a/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/MetricsProxyEndpoint.java +++ b/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/MetricsProxyEndpoint.java @@ -26,6 +26,7 @@ import java.nio.channels.Channels; import java.nio.channels.ReadableByteChannel; import java.nio.channels.WritableByteChannel; +import java.util.regex.Pattern; import javax.inject.Inject; import javax.servlet.http.HttpServletResponse; import javax.ws.rs.DefaultValue; @@ -47,6 +48,9 @@ @Path("/metrics") public class MetricsProxyEndpoint { + private static final Pattern SAFE_API_PATTERN = + Pattern.compile("^[a-zA-Z0-9_]+$"); + private MetricsServiceProvider metricsServiceProvider; @Inject @@ -67,6 +71,11 @@ public void getMetricsResponse( @Context UriInfo uriInfo, @Context HttpServletResponse httpServletResponse ) throws Exception { + if (!SAFE_API_PATTERN.matcher(api).matches()) { + httpServletResponse.sendError(HttpServletResponse.SC_BAD_REQUEST, + "Invalid metrics API name."); + return; + } if (metricsServiceProvider != null) { HttpURLConnection connection = metricsServiceProvider.getMetricsResponse( api, uriInfo.getRequestUri().getQuery());