Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix arbitrary file upload vulnerability with httpServerEnabled #3982

Merged
merged 3 commits into from
Jun 19, 2023
Merged

Fix arbitrary file upload vulnerability with httpServerEnabled #3982

merged 3 commits into from
Jun 19, 2023

Conversation

Shawyeok
Copy link
Contributor

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.

bookkeeper/bookkeeper-http/vertx-http-server/src/main/java/org/apache/bookkeeper/http/vertx/VertxHttpServer.java

Line 82 in 7f64246

router.route().handler(BodyHandler.create());

This simple command will upload a file into the file-uploads directory under the bookkeeper server process CWD.

$ 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)).

@Shawyeok Shawyeok changed the title Fix vertx httpserver arbitrary file upload vulnerability Fix arbitrary file upload vulnerability with httpServerEnabled Jun 11, 2023
wenbingshen
wenbingshen approved these changes Jun 11, 2023
View reviewed changes
Copy link
Member

@wenbingshen wenbingshen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change make sense to me.

@zymap zymap requested a review from eolivelli June 12, 2023 01:22
@zymap zymap requested a review from hangc0276 June 12, 2023 01:23
@zymap
Copy link
Member

zymap commented Jun 12, 2023

@Shawyeok Please fix the checkstyle issue in your code.

@Shawyeok
Copy link
Contributor Author

Shawyeok commented Jun 12, 2023 •
edited

@Shawyeok Please fix the checkstyle issue in your code.

Fixed, PTAL

@zymap zymap added this to the 4.17.0 milestone Jun 12, 2023
@hangc0276 hangc0276 merged commit 1809e69 into apache:master Jun 19, 2023
16 checks passed
zymap pushed a commit that referenced this pull request Jun 19, 2023
@Shawyeok @zymap
Fix arbitrary file upload vulnerability with httpServerEnabled (#3982)
07d3af6 
### 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 1809e69)
hangc0276 pushed a commit to hangc0276/bookkeeper that referenced this pull request Jun 26, 2023
@Shawyeok @hangc0276
Fix arbitrary file upload vulnerability with httpServerEnabled (apach…
7cfff56 
…e#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 1809e69)
zymap pushed a commit that referenced this pull request Dec 7, 2023
@Shawyeok @zymap
Fix arbitrary file upload vulnerability with httpServerEnabled (#3982)
7ee55fb 
### 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 1809e69)
Ghatage pushed a commit to sijie/bookkeeper that referenced this pull request Jul 12, 2024
@Shawyeok
Fix arbitrary file upload vulnerability with httpServerEnabled (apach…
29d2d62 
…e#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)`).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hangc0276 hangc0276 hangc0276 approved these changes

@dlg99 dlg99 dlg99 approved these changes

@shoothzj shoothzj shoothzj approved these changes

@zymap zymap zymap approved these changes

@wenbingshen wenbingshen wenbingshen approved these changes

@eolivelli eolivelli Awaiting requested review from eolivelli

Assignees

@Shawyeok Shawyeok

Labels
area/http cherry-picked/branch-4.14 cherry-picked/branch-4.15 cherry-picked/branch-4.16 release/4.14.8 release/4.15.5 release/4.16.2 type/bug
Projects
None yet
Milestone
4.17.0
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@Shawyeok @zymap @hangc0276 @dlg99 @shoothzj @wenbingshen