-
Notifications
You must be signed in to change notification settings - Fork 18.6k
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
gitutils: add validation for ref (CVE-2019-13139) #38944
Conversation
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com> (cherry picked from commit 723b107ca4fba14580a6cd971e63d8af2e7d2bbe) Signed-off-by: Andrew Hsu <andrewhsu@docker.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Codecov Report
@@ Coverage Diff @@
## master #38944 +/- ##
=========================================
Coverage ? 36.9%
=========================================
Files ? 614
Lines ? 45404
Branches ? 0
=========================================
Hits ? 16757
Misses ? 26357
Partials ? 2290 |
relevant changes; - moby/moby#38006 / docker-archive/engine#114 client: use io.LimitedReader for reading HTTP error - moby/moby#38634 / docker-archive/engine#167 pkg/archive:CopyTo(): fix for long dest filename - fixes docker/for-linux#484 for 18.09 - moby/moby#38944 / docker-archive/engine#183 gitutils: add validation for ref - moby/moby#37780 / docker-archive/engine#55 pkg/progress: work around closing closed channel panic - addresses moby/moby#/37735 pkg/progress: panic due to race on shutdown Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
relevant changes; - moby/moby#38006 / docker-archive/engine#114 client: use io.LimitedReader for reading HTTP error - moby/moby#38634 / docker-archive/engine#167 pkg/archive:CopyTo(): fix for long dest filename - fixes docker/for-linux#484 for 18.09 - moby/moby#38944 / docker-archive/engine#183 gitutils: add validation for ref - moby/moby#37780 / docker-archive/engine#55 pkg/progress: work around closing closed channel panic - addresses moby/moby#/37735 pkg/progress: panic due to race on shutdown Signed-off-by: Sebastiaan van Stijn <github@gone.nl> Upstream-commit: 010c234a0d5a03d450ebec60be37dd9f279feeca Component: cli
For reference I requested a CVE and CVE-2019-13139 has been reserved for this issue. |
@staaldraad thanks! I got notified of the CVE, so updated the titles of all the related PR's to include it 👍 We should probably update the release notes as well; https://github.com/docker/docker.github.io/blob/master/engine/release-notes.md#18094 Let me know if you're interested in opening a pull request in that repository, or if you want me to do so |
Thanks @thaJeztah 🎉 I've opened a PR -- wanted to tag you in as reviewer but it seems like labels and reviewers aren't available to non-project members |
From a fix that @tonistiigi created, this PR adds validation for git ref so it can't be misinterpreted as a flag.
fetch --
is a cleaner option but as it is theoretically possible to also hit it in checkout there's a custom validation as well.Thanks to @staaldraad for pointing this issue out originally.
cc @justincormack