f9483df5af
but was merged after the 1.11.0 merge window. CVE-2016-3697. bsc#976777.
+ cve-2016-3697-numeric-uid.patch
The upstream PR is here[1] and was vendored into Docker here[2].
[1]: https://github.com/opencontainers/runc/pull/708
[2]: https://github.com/docker/docker/pull/21665
OBS-URL: https://build.opensuse.org/package/show/Virtualization:containers/docker?expand=0&rev=101
317 lines
11 KiB
Diff
317 lines
11 KiB
Diff
From d67c0bf0caf26358f5345d6c0ac039026c46bd1e Mon Sep 17 00:00:00 2001
|
|
From: Aleksa Sarai <asarai@suse.de>
|
|
Date: Fri, 22 Apr 2016 20:36:43 +1000
|
|
Subject: [PATCH] libcontainer: user: always treat numeric ids numerically
|
|
|
|
Most shadow-related tools don't treat numeric ids as potential
|
|
usernames, so change our behaviour to match that. Previously, using an
|
|
explicit specification like 111:222 could result in the UID and GID not
|
|
being 111 and 222 respectively (which is confusing).
|
|
|
|
Some of the code was quite confusing inside libcontainer/user, so
|
|
refactor and comment it so future maintainers can understand what's
|
|
going and what edge cases we have to deal with.
|
|
|
|
This fixes CVE-2016-3697.
|
|
|
|
Signed-off-by: Aleksa Sarai <asarai@suse.de>
|
|
---
|
|
.../runc/libcontainer/user/lookup.go | 3 +
|
|
.../opencontainers/runc/libcontainer/user/user.go | 149 ++++++++++++---------
|
|
2 files changed, 89 insertions(+), 63 deletions(-)
|
|
|
|
diff --git a/vendor/src/github.com/opencontainers/runc/libcontainer/user/lookup.go b/vendor/src/github.com/opencontainers/runc/libcontainer/user/lookup.go
|
|
index 6f8a982..7062940 100644
|
|
--- a/vendor/src/github.com/opencontainers/runc/libcontainer/user/lookup.go
|
|
+++ b/vendor/src/github.com/opencontainers/runc/libcontainer/user/lookup.go
|
|
@@ -9,6 +9,9 @@ import (
|
|
var (
|
|
// The current operating system does not provide the required data for user lookups.
|
|
ErrUnsupported = errors.New("user lookup: operating system does not provide passwd-formatted data")
|
|
+ // No matching entries found in file.
|
|
+ ErrNoPasswdEntries = errors.New("no matching entries in passwd file")
|
|
+ ErrNoGroupEntries = errors.New("no matching entries in group file")
|
|
)
|
|
|
|
func lookupUser(filter func(u User) bool) (User, error) {
|
|
diff --git a/vendor/src/github.com/opencontainers/runc/libcontainer/user/user.go b/vendor/src/github.com/opencontainers/runc/libcontainer/user/user.go
|
|
index e6375ea..43fd39e 100644
|
|
--- a/vendor/src/github.com/opencontainers/runc/libcontainer/user/user.go
|
|
+++ b/vendor/src/github.com/opencontainers/runc/libcontainer/user/user.go
|
|
@@ -15,7 +15,7 @@ const (
|
|
)
|
|
|
|
var (
|
|
- ErrRange = fmt.Errorf("Uids and gids must be in range %d-%d", minId, maxId)
|
|
+ ErrRange = fmt.Errorf("uids and gids must be in range %d-%d", minId, maxId)
|
|
)
|
|
|
|
type User struct {
|
|
@@ -42,29 +42,30 @@ func parseLine(line string, v ...interface{}) {
|
|
|
|
parts := strings.Split(line, ":")
|
|
for i, p := range parts {
|
|
+ // Ignore cases where we don't have enough fields to populate the arguments.
|
|
+ // Some configuration files like to misbehave.
|
|
if len(v) <= i {
|
|
- // if we have more "parts" than we have places to put them, bail for great "tolerance" of naughty configuration files
|
|
break
|
|
}
|
|
|
|
+ // Use the type of the argument to figure out how to parse it, scanf() style.
|
|
+ // This is legit.
|
|
switch e := v[i].(type) {
|
|
case *string:
|
|
- // "root", "adm", "/bin/bash"
|
|
*e = p
|
|
case *int:
|
|
- // "0", "4", "1000"
|
|
- // ignore string to int conversion errors, for great "tolerance" of naughty configuration files
|
|
+ // "numbers", with conversion errors ignored because of some misbehaving configuration files.
|
|
*e, _ = strconv.Atoi(p)
|
|
case *[]string:
|
|
- // "", "root", "root,adm,daemon"
|
|
+ // Comma-separated lists.
|
|
if p != "" {
|
|
*e = strings.Split(p, ",")
|
|
} else {
|
|
*e = []string{}
|
|
}
|
|
default:
|
|
- // panic, because this is a programming/logic error, not a runtime one
|
|
- panic("parseLine expects only pointers! argument " + strconv.Itoa(i) + " is not a pointer!")
|
|
+ // Someone goof'd when writing code using this function. Scream so they can hear us.
|
|
+ panic(fmt.Sprintf("parseLine only accepts {*string, *int, *[]string} as arguments! %#v is not a pointer!", e))
|
|
}
|
|
}
|
|
}
|
|
@@ -106,8 +107,8 @@ func ParsePasswdFilter(r io.Reader, filter func(User) bool) ([]User, error) {
|
|
return nil, err
|
|
}
|
|
|
|
- text := strings.TrimSpace(s.Text())
|
|
- if text == "" {
|
|
+ line := strings.TrimSpace(s.Text())
|
|
+ if line == "" {
|
|
continue
|
|
}
|
|
|
|
@@ -117,10 +118,7 @@ func ParsePasswdFilter(r io.Reader, filter func(User) bool) ([]User, error) {
|
|
// root:x:0:0:root:/root:/bin/bash
|
|
// adm:x:3:4:adm:/var/adm:/bin/false
|
|
p := User{}
|
|
- parseLine(
|
|
- text,
|
|
- &p.Name, &p.Pass, &p.Uid, &p.Gid, &p.Gecos, &p.Home, &p.Shell,
|
|
- )
|
|
+ parseLine(line, &p.Name, &p.Pass, &p.Uid, &p.Gid, &p.Gecos, &p.Home, &p.Shell)
|
|
|
|
if filter == nil || filter(p) {
|
|
out = append(out, p)
|
|
@@ -135,6 +133,7 @@ func ParseGroupFile(path string) ([]Group, error) {
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
+
|
|
defer group.Close()
|
|
return ParseGroup(group)
|
|
}
|
|
@@ -178,10 +177,7 @@ func ParseGroupFilter(r io.Reader, filter func(Group) bool) ([]Group, error) {
|
|
// root:x:0:root
|
|
// adm:x:4:root,adm,daemon
|
|
p := Group{}
|
|
- parseLine(
|
|
- text,
|
|
- &p.Name, &p.Pass, &p.Gid, &p.List,
|
|
- )
|
|
+ parseLine(text, &p.Name, &p.Pass, &p.Gid, &p.List)
|
|
|
|
if filter == nil || filter(p) {
|
|
out = append(out, p)
|
|
@@ -192,9 +188,10 @@ func ParseGroupFilter(r io.Reader, filter func(Group) bool) ([]Group, error) {
|
|
}
|
|
|
|
type ExecUser struct {
|
|
- Uid, Gid int
|
|
- Sgids []int
|
|
- Home string
|
|
+ Uid int
|
|
+ Gid int
|
|
+ Sgids []int
|
|
+ Home string
|
|
}
|
|
|
|
// GetExecUserPath is a wrapper for GetExecUser. It reads data from each of the
|
|
@@ -235,12 +232,12 @@ func GetExecUserPath(userSpec string, defaults *ExecUser, passwdPath, groupPath
|
|
// * "uid:gid
|
|
// * "user:gid"
|
|
// * "uid:group"
|
|
+//
|
|
+// It should be noted that if you specify a numeric user or group id, they will
|
|
+// not be evaluated as usernames (only the metadata will be filled). So attempting
|
|
+// to parse a user with user.Name = "1337" will produce the user with a UID of
|
|
+// 1337.
|
|
func GetExecUser(userSpec string, defaults *ExecUser, passwd, group io.Reader) (*ExecUser, error) {
|
|
- var (
|
|
- userArg, groupArg string
|
|
- name string
|
|
- )
|
|
-
|
|
if defaults == nil {
|
|
defaults = new(ExecUser)
|
|
}
|
|
@@ -258,87 +255,113 @@ func GetExecUser(userSpec string, defaults *ExecUser, passwd, group io.Reader) (
|
|
user.Sgids = []int{}
|
|
}
|
|
|
|
- // allow for userArg to have either "user" syntax, or optionally "user:group" syntax
|
|
+ // Allow for userArg to have either "user" syntax, or optionally "user:group" syntax
|
|
+ var userArg, groupArg string
|
|
parseLine(userSpec, &userArg, &groupArg)
|
|
|
|
+ // Convert userArg and groupArg to be numeric, so we don't have to execute
|
|
+ // Atoi *twice* for each iteration over lines.
|
|
+ uidArg, uidErr := strconv.Atoi(userArg)
|
|
+ gidArg, gidErr := strconv.Atoi(groupArg)
|
|
+
|
|
+ // Find the matching user.
|
|
users, err := ParsePasswdFilter(passwd, func(u User) bool {
|
|
if userArg == "" {
|
|
+ // Default to current state of the user.
|
|
return u.Uid == user.Uid
|
|
}
|
|
- return u.Name == userArg || strconv.Itoa(u.Uid) == userArg
|
|
+
|
|
+ if uidErr == nil {
|
|
+ // If the userArg is numeric, always treat it as a UID.
|
|
+ return uidArg == u.Uid
|
|
+ }
|
|
+
|
|
+ return u.Name == userArg
|
|
})
|
|
+
|
|
+ // If we can't find the user, we have to bail.
|
|
if err != nil && passwd != nil {
|
|
if userArg == "" {
|
|
userArg = strconv.Itoa(user.Uid)
|
|
}
|
|
- return nil, fmt.Errorf("Unable to find user %v: %v", userArg, err)
|
|
+ return nil, fmt.Errorf("unable to find user %s: %v", userArg, err)
|
|
}
|
|
|
|
- haveUser := users != nil && len(users) > 0
|
|
- if haveUser {
|
|
- // if we found any user entries that matched our filter, let's take the first one as "correct"
|
|
- name = users[0].Name
|
|
+ var matchedUserName string
|
|
+ if len(users) > 0 {
|
|
+ // First match wins, even if there's more than one matching entry.
|
|
+ matchedUserName = users[0].Name
|
|
user.Uid = users[0].Uid
|
|
user.Gid = users[0].Gid
|
|
user.Home = users[0].Home
|
|
} else if userArg != "" {
|
|
- // we asked for a user but didn't find them... let's check to see if we wanted a numeric user
|
|
- user.Uid, err = strconv.Atoi(userArg)
|
|
- if err != nil {
|
|
- // not numeric - we have to bail
|
|
- return nil, fmt.Errorf("Unable to find user %v", userArg)
|
|
+ // If we can't find a user with the given username, the only other valid
|
|
+ // option is if it's a numeric username with no associated entry in passwd.
|
|
+
|
|
+ if uidErr != nil {
|
|
+ // Not numeric.
|
|
+ return nil, fmt.Errorf("unable to find user %s: %v", userArg, ErrNoPasswdEntries)
|
|
}
|
|
+ user.Uid = uidArg
|
|
|
|
// Must be inside valid uid range.
|
|
if user.Uid < minId || user.Uid > maxId {
|
|
return nil, ErrRange
|
|
}
|
|
|
|
- // if userArg couldn't be found in /etc/passwd but is numeric, just roll with it - this is legit
|
|
+ // Okay, so it's numeric. We can just roll with this.
|
|
}
|
|
|
|
- if groupArg != "" || name != "" {
|
|
+ // On to the groups. If we matched a username, we need to do this because of
|
|
+ // the supplementary group IDs.
|
|
+ if groupArg != "" || matchedUserName != "" {
|
|
groups, err := ParseGroupFilter(group, func(g Group) bool {
|
|
- // Explicit group format takes precedence.
|
|
- if groupArg != "" {
|
|
- return g.Name == groupArg || strconv.Itoa(g.Gid) == groupArg
|
|
+ // If the group argument isn't explicit, we'll just search for it.
|
|
+ if groupArg == "" {
|
|
+ // Check if user is a member of this group.
|
|
+ for _, u := range g.List {
|
|
+ if u == matchedUserName {
|
|
+ return true
|
|
+ }
|
|
+ }
|
|
+ return false
|
|
}
|
|
|
|
- // Check if user is a member.
|
|
- for _, u := range g.List {
|
|
- if u == name {
|
|
- return true
|
|
- }
|
|
+ if gidErr == nil {
|
|
+ // If the groupArg is numeric, always treat it as a GID.
|
|
+ return gidArg == g.Gid
|
|
}
|
|
|
|
- return false
|
|
+ return g.Name == groupArg
|
|
})
|
|
if err != nil && group != nil {
|
|
- return nil, fmt.Errorf("Unable to find groups for user %v: %v", users[0].Name, err)
|
|
+ return nil, fmt.Errorf("unable to find groups for spec %v: %v", matchedUserName, err)
|
|
}
|
|
|
|
- haveGroup := groups != nil && len(groups) > 0
|
|
+ // Only start modifying user.Gid if it is in explicit form.
|
|
if groupArg != "" {
|
|
- if haveGroup {
|
|
- // if we found any group entries that matched our filter, let's take the first one as "correct"
|
|
+ if len(groups) > 0 {
|
|
+ // First match wins, even if there's more than one matching entry.
|
|
user.Gid = groups[0].Gid
|
|
- } else {
|
|
- // we asked for a group but didn't find id... let's check to see if we wanted a numeric group
|
|
- user.Gid, err = strconv.Atoi(groupArg)
|
|
- if err != nil {
|
|
- // not numeric - we have to bail
|
|
- return nil, fmt.Errorf("Unable to find group %v", groupArg)
|
|
+ } else if groupArg != "" {
|
|
+ // If we can't find a group with the given name, the only other valid
|
|
+ // option is if it's a numeric group name with no associated entry in group.
|
|
+
|
|
+ if gidErr != nil {
|
|
+ // Not numeric.
|
|
+ return nil, fmt.Errorf("unable to find group %s: %v", groupArg, ErrNoGroupEntries)
|
|
}
|
|
+ user.Gid = gidArg
|
|
|
|
- // Ensure gid is inside gid range.
|
|
+ // Must be inside valid gid range.
|
|
if user.Gid < minId || user.Gid > maxId {
|
|
return nil, ErrRange
|
|
}
|
|
|
|
- // if groupArg couldn't be found in /etc/group but is numeric, just roll with it - this is legit
|
|
+ // Okay, so it's numeric. We can just roll with this.
|
|
}
|
|
- } else if haveGroup {
|
|
- // If implicit group format, fill supplementary gids.
|
|
+ } else if len(groups) > 0 {
|
|
+ // Supplementary group ids only make sense if in the implicit form.
|
|
user.Sgids = make([]int, len(groups))
|
|
for i, group := range groups {
|
|
user.Sgids[i] = group.Gid
|
|
--
|
|
2.8.1
|
|
|