Skip to content

Commit c66bd2d

Browse files
committed
user: prevent falling back to looking up numeric usernames
Improve handling of numeric user/group to prevent looking up numeric values as usernames. This fixes a similar issue as CVE-2026-46680 in containerd. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
1 parent c873359 commit c66bd2d

2 files changed

Lines changed: 59 additions & 24 deletions

File tree

user/user.go

Lines changed: 45 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,38 @@ func GetExecUserPath(userSpec string, defaults *ExecUser, passwdPath, groupPath
259259
return GetExecUser(userSpec, defaults, passwd, group)
260260
}
261261

262+
// parseNumeric parses the given UID or GID value to an integer and within
263+
// the minID - maxID range.
264+
//
265+
// While the Linux kernel allows the max UID to be MaxUint32 - 2,
266+
// and the OCI Runtime Spec has no definition about the max UID, we require
267+
// the UID to be <= MaxInt32.
268+
//
269+
// See https://github.com/containerd/containerd/commit/de1341c201ffb0effebbf51d00376181968c8779
270+
func parseNumeric(val string) (int, bool, error) {
271+
if val == "" {
272+
return 0, false, nil
273+
}
274+
id, err := strconv.Atoi(val)
275+
if err != nil {
276+
if errors.Is(err, strconv.ErrSyntax) {
277+
// Discard the error, because non-numeric values are expected
278+
// when passing a username or group-name.
279+
return 0, false, nil
280+
}
281+
if errors.Is(err, strconv.ErrRange) {
282+
return 0, true, ErrRange
283+
}
284+
// Other errors ("invalid base", "invalid bit size"); should never
285+
// happen with strconv.Atoi.
286+
return 0, false, err
287+
}
288+
if id < minID || id > maxID {
289+
return 0, true, ErrRange
290+
}
291+
return id, true, nil
292+
}
293+
262294
// GetExecUser parses a user specification string (using the passwd and group
263295
// readers as sources for /etc/passwd and /etc/group data, respectively). In
264296
// the case of blank fields or missing data from the sources, the values in
@@ -304,8 +336,14 @@ func GetExecUser(userSpec string, defaults *ExecUser, passwd, group io.Reader) (
304336

305337
// Convert userArg and groupArg to be numeric, so we don't have to execute
306338
// Atoi *twice* for each iteration over lines.
307-
uidArg, uidErr := strconv.Atoi(userArg)
308-
gidArg, gidErr := strconv.Atoi(groupArg)
339+
uidArg, isUID, err := parseNumeric(userArg)
340+
if err != nil {
341+
return nil, err
342+
}
343+
gidArg, isGID, err := parseNumeric(groupArg)
344+
if err != nil {
345+
return nil, err
346+
}
309347

310348
// Find the matching user.
311349
users, err := ParsePasswdFilter(passwd, func(u User) bool {
@@ -314,8 +352,8 @@ func GetExecUser(userSpec string, defaults *ExecUser, passwd, group io.Reader) (
314352
return u.Uid == user.Uid
315353
}
316354

317-
if uidErr == nil {
318-
// If the userArg is numeric, always treat it as a UID.
355+
if isUID {
356+
// If the userArg is a valid numeric value, always treat it as a UID.
319357
return uidArg == u.Uid
320358
}
321359

@@ -341,18 +379,11 @@ func GetExecUser(userSpec string, defaults *ExecUser, passwd, group io.Reader) (
341379
// If we can't find a user with the given username, the only other valid
342380
// option is if it's a numeric username with no associated entry in passwd.
343381

344-
if uidErr != nil {
382+
if !isUID {
345383
// Not numeric.
346384
return nil, fmt.Errorf("unable to find user %s: %w", userArg, ErrNoPasswdEntries)
347385
}
348386
user.Uid = uidArg
349-
350-
// Must be inside valid uid range.
351-
if user.Uid < minID || user.Uid > maxID {
352-
return nil, ErrRange
353-
}
354-
355-
// Okay, so it's numeric. We can just roll with this.
356387
}
357388

358389
// On to the groups. If we matched a username, we need to do this because of
@@ -370,7 +401,7 @@ func GetExecUser(userSpec string, defaults *ExecUser, passwd, group io.Reader) (
370401
return false
371402
}
372403

373-
if gidErr == nil {
404+
if isGID {
374405
// If the groupArg is numeric, always treat it as a GID.
375406
return gidArg == g.Gid
376407
}
@@ -390,18 +421,11 @@ func GetExecUser(userSpec string, defaults *ExecUser, passwd, group io.Reader) (
390421
// If we can't find a group with the given name, the only other valid
391422
// option is if it's a numeric group name with no associated entry in group.
392423

393-
if gidErr != nil {
424+
if !isGID {
394425
// Not numeric.
395426
return nil, fmt.Errorf("unable to find group %s: %w", groupArg, ErrNoGroupEntries)
396427
}
397428
user.Gid = gidArg
398-
399-
// Must be inside valid gid range.
400-
if user.Gid < minID || user.Gid > maxID {
401-
return nil, ErrRange
402-
}
403-
404-
// Okay, so it's numeric. We can just roll with this.
405429
}
406430
} else if len(groups) > 0 {
407431
// Supplementary group ids only make sense if in the implicit form.

user/user_test.go

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ adm:x:42:43:adm:/var/adm:/bin/false
106106
odd:x:111:112::/home/odd:::::
107107
2147483647:x:0:0:maxint32:/root:/bin/bash
108108
2147483648:x:0:0:toolarge:/root:/bin/bash
109+
9223372036854775807:x:0:0:maxint64:/root:/bin/bash
109110
user7456:x:7456:100:Vasya:/home/user7456
110111
this is just some garbage data
111112
`
@@ -117,6 +118,7 @@ grp:x:1234:root,adm,user7456
117118
odd:x:444:
118119
2147483647:x:1235:
119120
2147483648:x:1236:
121+
9223372036854775807:x:1237:
120122
this is just some garbage data
121123
` + largeGroup()
122124

@@ -293,6 +295,8 @@ adm:x:42:43:adm:/var/adm:/bin/false
293295
-42:x:12:13:broken:/very/broken
294296
2147483647:x:0:0:maxint32:/root:/bin/bash
295297
2147483648:x:0:0:toolarge:/root:/bin/bash
298+
9223372036854775807:x:0:0:maxint64:/root:/bin/bash
299+
9223372036854775808:x:0:0:maxint64plusone:/root:/bin/bash
296300
this is just some garbage data
297301
`
298302
const groupContent = `
@@ -301,6 +305,8 @@ adm:x:43:
301305
grp:x:1234:root,adm
302306
2147483647:x:1235:
303307
2147483648:x:1236:
308+
9223372036854775807:x:1237:
309+
9223372036854775808:x:1238:
304310
this is just some garbage data
305311
`
306312

@@ -319,9 +325,14 @@ this is just some garbage data
319325
"-5:-2",
320326
"-42",
321327
"-43",
322-
"42:2147483648", // maxID + 1
323-
"2147483648:43", // maxID + 1
324-
"2147483648", // maxID + 1
328+
"42:2147483648", // maxID + 1
329+
"2147483648:43", // maxID + 1
330+
"2147483648", // maxID + 1
331+
"7456:9223372036854775807", // maxInt64
332+
"9223372036854775807:43", // maxInt64
333+
"9223372036854775807", // maxInt64
334+
"9223372036854775808", // maxInt64+1, must not resolve as username
335+
"0:9223372036854775808", // maxInt64+1, must not resolve as group name
325336
}
326337

327338
for _, tc := range tests {

0 commit comments

Comments
 (0)