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

security: fix write-to-RAM DoS vulnerability #5957

Merged
merged 1 commit into from May 18, 2018
Merged

Conversation

aead
Copy link
Member

@aead aead commented May 18, 2018

Description

This commit fixes a DoS vulnerability for certain APIs using
signature V4 by verifying the content-md5 and/or content-sha56 of
the request body in a streaming mode.

The issue was caused by reading the entire body of the request into
memory to verify the content-md5 or content-sha56 checksum if present.

The vulnerability could be exploited by either replaying a V4 request
(in the 15 min time frame) or sending a V4 presigned request with a
large body.

Motivation and Context

Security, DoS

How Has This Been Tested?

manually

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added unit tests to cover my changes.
  • I have added/updated functional tests in mint. (If yes, add mint PR # here: )
  • All new and existing tests passed.

@donatello
Copy link
Member

Fix looks good, though not sure why the build choked.

@aead
Copy link
Member Author

aead commented May 18, 2018

@donatello
"Bug" in hash.Reader:
Missing

if size >= 0 {
	src = io.LimitReader(src, size)
}

We limited the size of the request to 0...
Fixed it.

This commit fixes a DoS vulnerability for certain APIs using
signature V4 by verifying the content-md5 and/or content-sha56 of
the request body in a streaming mode.

The issue was caused by reading the entire body of the request into
memory to verify the content-md5 or content-sha56 checksum if present.

The vulnerability could be exploited by either replaying a V4 request
(in the 15 min time frame) or sending a V4 presigned request with a
large body.
@codecov
Copy link

codecov bot commented May 18, 2018

Codecov Report

Merging #5957 into master will decrease coverage by 0.01%.
The diff coverage is 60%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5957      +/-   ##
==========================================
- Coverage   59.62%   59.61%   -0.02%     
==========================================
  Files         214      214              
  Lines       30770    30758      -12     
==========================================
- Hits        18347    18335      -12     
- Misses      10877    10878       +1     
+ Partials     1546     1545       -1
Impacted Files Coverage Δ
pkg/hash/reader.go 100% <100%> (ø) ⬆️
cmd/auth-handler.go 82.82% <54.54%> (+0.9%) ⬆️
cmd/xl-v1-common.go 81.66% <0%> (-3.34%) ⬇️
cmd/xl-sets.go 58.6% <0%> (-0.22%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1cf381f...b683f00. Read the comment docs.

@deekoder deekoder merged commit 9c8b730 into minio:master May 18, 2018
@aead aead deleted the DoS-v4 branch June 5, 2018 11:31
@Rajik

This comment has been minimized.

@nitisht

This comment has been minimized.

@minio minio locked as resolved and limited conversation to collaborators Nov 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants