specconv: do not permit null bytes in mount fields #3287
Conversation
| strings.Contains(mnt.Source, "\x00") || | ||
| strings.Contains(mnt.Device, "\x00") || | ||
| strings.Contains(mnt.Data, "\x00") { | ||
| return nil, errors.New("mount field contains null byte") |
There was a problem hiding this comment.
would it be useful to include which field (for users to debug what's wrong)?
In we think it's useful, we could do a loop over the fields, and check each one
There was a problem hiding this comment.
That said; it would still return the first error (unless we always check all of them), so usefulness may be limited
There was a problem hiding this comment.
This is more of a security measure, and less a mechanism to tell the user to correct something. If there's a null byte in there, something is very wrong.
There was a problem hiding this comment.
Yeah the main reason for doing this in specconv is to make sure we always reject such configurations, rather than only rejecting them when we hit the actually attack-prevention check in bootstrapData.
There was a problem hiding this comment.
Yes, it was mostly a nit; I don't think "security measure" and "informative/detailed" error is mutually exclusive, but this situation should be a corner-case, so not really worth spending too much time on to make it "nice".
kolyshkin
added
backport/1.0-todo
Using null bytes as control characters for sending strings via netlink opens us up to a user explicitly putting a null byte in a mount string (which JSON will happily let you do) and then causing us to open a mount path different to the one expected. In practice this is more of an issue in an environment such as Kubernetes where you may have path-based access control policies (which are more susceptible to these kinds of flaws). Found by Google Project Zero. Fixes: 9c44407 ("Open bind mount sources from the host userns") Reported-by: Felix Wilhelm <fwilhelm@google.com> Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Using null bytes as control characters for sending strings via netlink
opens us up to a user explicitly putting a null byte in a mount string
(which JSON will happily let you do) and then causing us to open a mount
path different to the one expected.
In practice this is more of an issue in an environment such as
Kubernetes where you may have path-based access control policies (which
are more susceptible to these kinds of flaws).
Found by Google Project Zero.
Fixes: 9c44407 ("Open bind mount sources from the host userns")
Reported-by: Felix Wilhelm fwilhelm@google.com
Signed-off-by: Aleksa Sarai cyphar@cyphar.com