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

[thaJeztah]

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]

/cc @justincormack @vdemeester

[thaJeztah]

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

[thaJeztah]

Updated; PTAL

[andrewhsu]

LGTM

[vdemeester]

LGTM 🐸

[ret2libc]

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]

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]

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]

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]

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]

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?

[zelivans]

@justincormack If I understand correctly, both CpusetCpus and CpusetMems can be used only for run, update and build commands. This should be correct for API v1.39 and 1.40. These commands should be treated as ones that can cause DoS on the host or dockerd regardless of this bug. So even if the API is narrowed down, the commands supporting this flag are still dangerous.

I guess what you were suggesting is that it may be possible to filter the previous commands to make them safe through plugins? E.g. limiting the run command's allowed parameters to let users run a container without giving them root-like permissions. If it is possible I guess it is fair to leave this as a CVE. It is only relevant to this edge case, though.