8000 [release/2.6] registry/{storage,handlers}: limit content sizes by stevvooe · Pull Request #2341 · distribution/distribution · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[release/2.6] registry/{storage,handlers}: limit content sizes #2341

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

Merged
merged 1 commit into from
Jul 20, 2017

Conversation

stevvooe
Copy link
Collaborator

Under certain circumstances, the use of StorageDriver.GetContent can
result in unbounded memory allocations. In particualr, this happens when
accessing a layer through the manifests endpoint.

This problem is mitigated by setting a 4MB limit when using to access
content that may have been accepted from a user. In practice, this means
setting the limit with the use of BlobProvider.Get by wrapping
StorageDriver.GetContent in a helper that uses StorageDriver.Reader
with a limitReader that returns an error.

When mitigating this security issue, we also noticed that the size of
manifests uploaded to the registry is also unlimited. We apply similar
logic to the request body of payloads that are full buffered.

Signed-off-by: Stephen J Day stephen.day@docker.com
(cherry picked from commit 55ea440)
Signed-off-by: Stephen J Day stephen.day@docker.com

cc @riyazdf @dmcgowan

Under certain circumstances, the use of `StorageDriver.GetContent` can
result in unbounded memory allocations. In particualr, this happens when
accessing a layer through the manifests endpoint.

This problem is mitigated by setting a 4MB limit when using to access
content that may have been accepted from a user. In practice, this means
setting the limit with the use of `BlobProvider.Get` by wrapping
`StorageDriver.GetContent` in a helper that uses `StorageDriver.Reader`
with a `limitReader` that returns an error.

When mitigating this security issue, we also noticed that the size of
manifests uploaded to the registry is also unlimited. We apply similar
logic to the request body of payloads that are full buffered.

Signed-off-by: Stephen J Day <stephen.day@docker.com>
(cherry picked from commit 55ea440)
Signed-off-by: Stephen J Day <stephen.day@docker.com>
Copy link
Contributor
@riyazdf riyazdf left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov
Copy link
codecov bot commented Jul 20, 2017

Codecov Report

Merging #2341 into release/2.6 will decrease coverage by 11.06%.
The diff coverage is 51.06%.

Impacted file tree graph

@@               Coverage Diff                @@
##           release/2.6    #2341       +/-   ##
================================================
- Coverage         61.1%   50.04%   -11.07%     
================================================
  Files              126      127        +1     
  Lines            11351    14342     +2991     
================================================
+ Hits              6936     7177      +241     
- Misses            3523     6410     +2887     
+ Partials           892      755      -137
Impacted Files Coverage Δ
registry/handlers/images.go 56.84% <0%> (-0.08%) ⬇️
registry/handlers/blobupload.go 45.2% <0%> (+0.02%) ⬆️
registry/storage/blobstore.go 55.63% <100%> (-1.51%) ⬇️
registry/handlers/helpers.go 39.53% <100%> (+0.24%) ⬆️
registry/storage/io.go 48.48% <48.48%> (ø)
registry/storage/driver/gcs/gcs.go 0.32% <0%> (-69.02%) ⬇️
registry/storage/driver/oss/oss.go 0.45% <0%> (-57.55%) ⬇️
registry/storage/driver/s3-aws/s3.go 3.73% <0%> (-57.13%) ⬇️
registry/storage/driver/s3-goamz/s3.go 0.4% <0%> (-50.74%) ⬇️
digest/digester.go 45% <0%> (-15%) ⬇️
... and 116 more

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 42ea75c...29fa466. Read the comment docs.

@stevvooe stevvooe merged commit c829241 into distribution:release/2.6 Jul 20, 2017
@stevvooe stevvooe deleted the limit-payload-size-26 branch July 20, 2017 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0