This is an automated email from the ASF dual-hosted git repository. chenhang pushed a commit to branch branch-4.14 in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
commit 7cfff5689de095e69474b7a05072cd9ecc8c32c4 Author: 萧易客 <[email protected]> AuthorDate: Mon Jun 19 10:19:15 2023 +0800 Fix arbitrary file upload vulnerability with httpServerEnabled (#3982) ### Motivation There is a potential arbitrary file upload vulnerability with `httpServerEnabled=true`, it's caused by `BodyHandler.create()` which returns a BodyHandler that automatically processes file upload requests. https://github.com/apache/bookkeeper/blob/7f64246ad38981126cc8dd929ff448805a738b8f/bookkeeper-http/vertx-http-server/src/main/java/org/apache/bookkeeper/http/vertx/VertxHttpServer.java#L82 This simple command will upload a file into the `file-uploads` directory under the bookkeeper server process `CWD`. ```shell $ curl -i --request POST \ --url http://localhost:8000/api/v1/bookie/info \ --header 'Content-Type: multipart/form-data' \ --form file=@<a-path-of-the-file> $ ls LICENSE NOTICE README.md bin conf deps file-uploads lib logs scripts $ ls file-uploads 758801ba-ea1e-49e3-85d6-e510f539ea0d ``` ### Changes Create the `BodyHandler` with handleFileUploads disabled (`BodyHandler.create(false)`). (cherry picked from commit 1809e69c6d4eeced49e76b76e979e9da96942231) --- .../bookkeeper/http/vertx/VertxHttpServer.java | 2 +- .../bookkeeper/http/vertx/TestVertxHttpServer.java | 76 ++++++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/bookkeeper-http/vertx-http-server/src/main/java/org/apache/bookkeeper/http/vertx/VertxHttpServer.java b/bookkeeper-http/vertx-http-server/src/main/java/org/apache/bookkeeper/http/vertx/VertxHttpServer.java index 8d4edd0944..baa7e7cf53 100644 --- a/bookkeeper-http/vertx-http-server/src/main/java/org/apache/bookkeeper/http/vertx/VertxHttpServer.java +++ b/bookkeeper-http/vertx-http-server/src/main/java/org/apache/bookkeeper/http/vertx/VertxHttpServer.java @@ -67,7 +67,7 @@ public class VertxHttpServer implements HttpServer { CompletableFuture<AsyncResult<io.vertx.core.http.HttpServer>> future = new CompletableFuture<>(); VertxHttpHandlerFactory handlerFactory = new VertxHttpHandlerFactory(httpServiceProvider); Router router = Router.router(vertx); - router.route().handler(BodyHandler.create()); + router.route().handler(BodyHandler.create(false)); HttpRouter<VertxAbstractHandler> requestRouter = new HttpRouter<VertxAbstractHandler>(handlerFactory) { @Override public void bindHandler(String endpoint, VertxAbstractHandler handler) { diff --git a/bookkeeper-http/vertx-http-server/src/test/java/org/apache/bookkeeper/http/vertx/TestVertxHttpServer.java b/bookkeeper-http/vertx-http-server/src/test/java/org/apache/bookkeeper/http/vertx/TestVertxHttpServer.java index 79667f7391..509cb3caf8 100644 --- a/bookkeeper-http/vertx-http-server/src/test/java/org/apache/bookkeeper/http/vertx/TestVertxHttpServer.java +++ b/bookkeeper-http/vertx-http-server/src/test/java/org/apache/bookkeeper/http/vertx/TestVertxHttpServer.java @@ -20,13 +20,20 @@ */ package org.apache.bookkeeper.http.vertx; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import com.google.common.io.Files; +import io.vertx.ext.web.handler.BodyHandler; import java.io.BufferedReader; +import java.io.File; +import java.io.FileInputStream; import java.io.IOException; import java.io.InputStreamReader; import java.io.OutputStream; +import java.io.OutputStreamWriter; +import java.io.PrintWriter; import java.net.HttpURLConnection; import java.net.URL; import java.nio.charset.StandardCharsets; @@ -97,6 +104,22 @@ public class TestVertxHttpServer { httpServer.stopServer(); } + @Test + public void testArbitraryFileUpload() throws IOException { + VertxHttpServer httpServer = new VertxHttpServer(); + HttpServiceProvider httpServiceProvider = NullHttpServiceProvider.getInstance(); + httpServer.initialize(httpServiceProvider); + assertTrue(httpServer.startServer(0)); + int port = httpServer.getListeningPort(); + File tempFile = File.createTempFile("test-" + System.currentTimeMillis(), null); + Files.asCharSink(tempFile, StandardCharsets.UTF_8).write(TestVertxHttpServer.class.getName()); + String[] filenamesBeforeUploadRequest = listFiles(BodyHandler.DEFAULT_UPLOADS_DIRECTORY); + HttpResponse httpResponse = sendFile(getUrl(port, HttpRouter.BOOKIE_INFO), tempFile); + assertEquals(HttpServer.StatusCode.OK.getValue(), httpResponse.responseCode); + assertArrayEquals(filenamesBeforeUploadRequest, listFiles(BodyHandler.DEFAULT_UPLOADS_DIRECTORY)); + httpServer.stopServer(); + } + private HttpResponse send(String url, HttpServer.Method method) throws IOException { return send(url, method, ""); } @@ -131,6 +154,59 @@ public class TestVertxHttpServer { return new HttpResponse(responseCode, response.toString()); } + private HttpResponse sendFile(String url, File file) throws IOException { + URL obj = new URL(url); + HttpURLConnection con = (HttpURLConnection) obj.openConnection(); + String boundary = "---------------------------" + System.currentTimeMillis(); + // optional, default is GET + con.setRequestMethod("POST"); + con.setDoOutput(true); + con.setRequestProperty("Content-Type", "multipart/form-data; boundary=" + boundary); + try ( + OutputStream outputStream = con.getOutputStream(); + PrintWriter writer = new PrintWriter(new OutputStreamWriter(outputStream, StandardCharsets.UTF_8), + true); + FileInputStream fileInputStream = new FileInputStream(file); + ) { + writer.append("--" + boundary).append("\r\n"); + writer.append("Content-Disposition: form-data; name=\"file\"; filename=\"file.txt\"").append("\r\n"); + writer.append("Content-Type: text/plain").append("\r\n"); + writer.append("\r\n"); + + byte[] buffer = new byte[4096]; + int bytesRead; + while ((bytesRead = fileInputStream.read(buffer)) != -1) { + outputStream.write(buffer, 0, bytesRead); + } + + writer.append("\r\n"); + writer.append("--" + boundary + "--").append("\r\n"); + } + int responseCode = con.getResponseCode(); + StringBuilder response = new StringBuilder(); + BufferedReader in = null; + try { + in = new BufferedReader(new InputStreamReader(con.getInputStream())); + String inputLine; + while ((inputLine = in.readLine()) != null) { + response.append(inputLine); + } + } finally { + if (in != null) { + in.close(); + } + } + return new HttpResponse(responseCode, response.toString()); + } + + private String[] listFiles(String directory) { + File directoryFile = new File(directory); + if (!directoryFile.exists() || !directoryFile.isDirectory()) { + return new String[0]; + } + return directoryFile.list(); + } + private String getUrl(int port, String path) { return "http://localhost:"; + port + path; }
