This is an automated email from the ASF dual-hosted git repository.
chenhang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
The following commit(s) were added to refs/heads/master by this push:
new 1809e69c6d Fix arbitrary file upload vulnerability with
httpServerEnabled (#3982)
1809e69c6d is described below
commit 1809e69c6d4eeced49e76b76e979e9da96942231
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)`).
---
.../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 ceaaf7d02d..331ed6566b 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
@@ -79,7 +79,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 24a4bf9185..a0c6c4ae69 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;
@@ -109,6 +116,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, "");
}
@@ -143,6 +166,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;
}
