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

[18.09] backport fix denial of service with large numbers in cpuset-cpus and cpuset-mems #70

Merged
merged 1 commit into from Oct 11, 2018

Conversation

Projects
None yet
6 participants
@thaJeztah
Copy link
Member

thaJeztah commented Oct 4, 2018

Backport of moby#37967 for 18.09

git checkout -b 18.09_backport_upstream_dos_fix ce-engine/18.09
git cherry-pick -s -S -x f8e876d7616469d07b8b049ecb48967eeb8fa7a5

cherry-pick was clean; no conflicts

Using a value such as --cpuset-mems=1-9223372036854775807 would cause
dockerd to run out of memory allocating a map of the values in the
validation code. Set limits to the normal limit of the number of CPUs,
and improve the error handling.

Reported by Huawei PSIRT.

- Description for the changelog

* Fix denial of service with large numbers in `--cpuset-cpus` and `--cpuset-mems`

@thaJeztah thaJeztah added this to the 18.09.0 milestone Oct 4, 2018

@thaJeztah

This comment has been minimized.

Copy link
Member Author

thaJeztah commented Oct 4, 2018

@thaJeztah

This comment has been minimized.

Copy link
Member Author

thaJeztah commented Oct 5, 2018

Need to update this one with the latest changes (after review comments on the upstream PR)

Fix denial of service with large numbers in cpuset-cpus and cpuset-mems
Using a value such as `--cpuset-mems=1-9223372036854775807` would cause
`dockerd` to run out of memory allocating a map of the values in the
validation code. Set limits to the normal limit of the number of CPUs,
and improve the error handling.

Reported by Huawei PSIRT.

Signed-off-by: Justin Cormack <justin.cormack@docker.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit f8e876d)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

@thaJeztah thaJeztah force-pushed the thaJeztah:18.09_backport_upstream_dos_fix branch from 73b5df7 to 0922d32 Oct 5, 2018

@thaJeztah

This comment has been minimized.

Copy link
Member Author

thaJeztah commented Oct 5, 2018

Updated; PTAL

@andrewhsu
Copy link

andrewhsu left a comment

LGTM

@vdemeester
Copy link
Member

vdemeester left a comment

LGTM 🐸

@tiborvass tiborvass merged commit 4b8336f into docker:18.09 Oct 11, 2018

5 checks passed

experimental Jenkins build Docker-PRs-experimental 42462 has succeeded
Details
janky Jenkins build Docker-PRs 51250 has succeeded
Details
powerpc Jenkins build Docker-PRs-powerpc 11745 has succeeded
Details
windowsRS1 Jenkins build Docker-PRs-WoW-RS1 22523 has succeeded
Details
z Jenkins build Docker-PRs-s390x 11537 has succeeded
Details

@thaJeztah thaJeztah deleted the thaJeztah:18.09_backport_upstream_dos_fix branch Oct 11, 2018

@ret2libc

This comment has been minimized.

Copy link

ret2libc commented Jan 29, 2019

This is CVE-2018-20699. However, I do not believe this issue deserves a CVE, as it does not allow an attacker to do anything he can't already do. To run such docker command you have to be root/high-privileged and if you are already root/high-privileged, there's no need to use this issue to stop dockerd or cause other more serious damages.

I'd like to ask MITRE to reject this flaw for the mentioned reasons. Anybody from upstream has a different opinion? Or if you are of the same idea, please do share your agreement to make MITRE decision easier.

@thaJeztah

This comment has been minimized.

Copy link
Member Author

thaJeztah commented Jan 29, 2019

Yes, if you have access to the Docker remote API, you're effectively root, so already have many other ways to cause a denial of service. But I'm not on the security-team, so let me /cc @justincormack here as well for input.

@justincormack

This comment has been minimized.

Copy link
Member

justincormack commented Jan 29, 2019

Lots of people run the Docker API with some lock down, and the denial of service is unexpected, so I don't think it makes sense to reject it totally, even if in many cases it is not important. Also our experience with getting MITRE to reject even obviously incorrect CVEs is not good.

@ret2libc

This comment has been minimized.

Copy link

ret2libc commented Jan 29, 2019

What do you mean by “some lock down”?
I think if upstream is ok with rejecting it MITRE shouldn’t make too many problems.

@justincormack

This comment has been minimized.

Copy link
Member

justincormack commented Jan 29, 2019

Our previous experience was we just managed to get a "disputed" note added https://www.cvedetails.com/cve/CVE-2016-6595/

By "some lock down" I mean authz plugins or other means of narrowing the API.

@ret2libc

This comment has been minimized.

Copy link

ret2libc commented Jan 29, 2019

By "some lock down" I mean authz plugins or other means of narrowing the API.

Plugins that would allow them to run a new container without giving them the root-like permissions to cause other similar issues?

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