Skip to content
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

User names beginning with 0x being interpreted as user identifiers #15985

Closed
0x2b3bfa0 opened this issue May 31, 2020 · 11 comments
Closed

User names beginning with 0x being interpreted as user identifiers #15985

0x2b3bfa0 opened this issue May 31, 2020 · 11 comments

Comments

@0x2b3bfa0
Copy link

0x2b3bfa0 commented May 31, 2020

Version the issue has been seen with

245 ea500ac and the current master d904afc

Used distribution

Ubuntu 20.04 LTS

Expected behaviour you didn't see

loginctl show-user $(id -u)
UID=1000
GID=1000
Name=0x2b3bfa0
Timestamp=Sat 2020-05-30 23:37:28 CEST
TimestampMonotonic=405537243
RuntimePath=/run/user/1000
Service=user@1000.service
Slice=user-1000.slice
Display=2
State=active
Sessions=16 2
IdleHint=no
IdleSinceHint=1590909501087020
IdleSinceHintMonotonic=5138907865
Linger=no

Unexpected behaviour you saw

loginctl show-user $(id -u)
Failed to get user: User ID 1000 is not logged in or lingering

Steps to reproduce the problem

  1. Create a user whose name starts with 0x or 0X (exempli gratia: 0x2b3bfa0).
  2. Log in with that user name and run loginctl show-user $(id -u).
  3. Check the unexpected behavior above.

Affected code

if (parse_uid(u, NULL) >= 0) /* Something that parses as numeric UID string is valid exactly when the

r = safe_atou32(s, &uid);

return safe_atou32_full(s, 0, (unsigned*) ret_u);

return safe_atou_full(s, base, (unsigned*) ret_u);

l = strtoul(s, &x, base);

Problem description
As per the strtoul(3) man page, when the base argument is 0, the string can also be interpreted as an hexadecimal or octal number:

If base is zero or 16, the string may then include a "0x" prefix, and the number will be read in base 16; otherwise, a zero base is taken as 10 (decimal) unless the next character is '0', in which case it is taken as 8 (octal).

Vulnerability

CVE-2020-13776: systemd through v245 mishandles numerical usernames such as ones composed of decimal digits or 0x followed by hex digits, as demonstrated by use of root privileges when privileges of the 0x0 user account were intended.

Related issues

@Malvineous
Copy link
Contributor

Adding the base parameter to strtoul() seems like it would be an incomplete fix. It would allow hex numbers to be recognised as string usernames, but you still wouldn't be able to use decimal numbers as a username, which apparently is allowed by the POSIX standard.

You could test for a string username before trying the parameter as a UID, but that can lead to unpredictable behaviour (e.g. create a username 0). It seems the only correct solution would be to require a flag on the command line when the caller wishes to specify a numeric UID rather than a username.

@0x2b3bfa0 0x2b3bfa0 mentioned this issue May 31, 2020
@0x2b3bfa0
Copy link
Author

0x2b3bfa0 commented May 31, 2020

POSIX.1-2017

3.437 User Name

A string that is used to identify a user; see also User Database. To be portable across systems conforming to POSIX.1-2017, the value is composed of characters from the portable filename character set. The <hyphen-minus> character should not be used as the first character of a portable user name.

3.282 Portable Filename Character Set

The set of characters from which portable filenames are constructed.

A B C D E F G H I J K L M N O P Q R S T U V W X Y Z
a b c d e f g h i j k l m n o p q r s t u v w x y z
0 1 2 3 4 5 6 7 8 9 . _ -

The last three characters are the <period>, <underscore>, and <hyphen-minus> characters, respectively. See also Pathname.

@0x2b3bfa0
Copy link
Author

0x2b3bfa0 commented May 31, 2020

@Malvineous, you're right: user names can be [a-zA-Z0-9._][a-zA-Z0-9._-]+, but it looks like systemd also wants to interpret user identifiers in the same fields.

@0x2b3bfa0
Copy link
Author

0x2b3bfa0 commented May 31, 2020

Adding the base parameter to strtoul() seems like it would be an incomplete fix. It would allow hex numbers to be recognised as string usernames, but you still wouldn't be able to use decimal numbers as a username, which apparently is allowed by the POSIX standard.

I don't know if such a drastic change is a good idea, as the underlying safe_atou_full function which calls strtoul is being used across all the codebase, and probably there are use cases where hexadecimal numbers are a valid input.

You could test for a string username before trying the parameter as a UID, but that can lead to unpredictable behaviour (e.g. create a username 0).

Or, better yet, check that the passed string is fully numeric before trying to interpret it as an user identifier, as per my suggestion in #15987. However, creating a username 0 might produce the same results as currently 0x0 or 0X0.

It seems the only correct solution would be to require a flag on the command line when the caller wishes to specify a numeric UID rather than a username.

I don't think so, as this code is being used in many more places (see #6237). If we intend to clearly differentiate user identifiers and user names, many breaking changes will be required.

@0x2b3bfa0
Copy link
Author

https://access.redhat.com/solutions/3103631 seems to provide a good insight on this issue

@Malvineous
Copy link
Contributor

The RedHat link is interesting, because it suggests that at some point 0day was fed into strtoul() and accepted as 0 without being checked further. I agree a command line parameter would be tricky if you also wanted it to work in unit files, as User=1234 for example.

I wonder whether you could pick a character that isn't allowed in a username, like '#' or '@' (can't use '#' because it often starts a comment though, not sure about '@'), and when that appears as the first character in the username, the rest of it is treated as a UID. Otherwise it's treated as a username if one exists, and only falls back to a UID if there is no matching username.

That would allow you to specify @0 if you wanted a UID for sure, otherwise 0day or 1234 would be chosen as usernames if they existed, falling back to UIDs only if matching usernames couldn't be found (thus giving you backwards compatibility).

You'd only run into problems with this solution if you were using a UID now, and you had a username on the system that matched that UID but itself had a different UID. Not sure why any sane person would do that, but the solution would be to just add '@' onto the front of the UID.

@0x2b3bfa0
Copy link
Author

0x2b3bfa0 commented May 31, 2020

The RedHat link is interesting, because it suggests that at some point 0day was fed into strtoul() and accepted as 0 without being checked further.

Yes it even links to #6237 here on GitHub: «[...] towards the end of the bug report [...]»

I agree a command line parameter would be tricky if you also wanted it to work in unit files, as User=1234 for example.

Certainly, your proposed solution would be more elegant and practical.

I wonder whether you could pick a character that isn't allowed in a username, like '#' or '@' (can't use '#' because it often starts a comment though, not sure about '@'), and when that appears as the first character in the username, the rest of it is treated as a UID. Otherwise it's treated as a username if one exists, and only falls back to a UID if there is no matching username.

That would allow you to specify @0 if you wanted a UID for sure, otherwise 0day or 1234 would be chosen as usernames if they existed, falling back to UIDs only if matching usernames couldn't be found (thus giving you backwards compatibility).

You'd only run into problems with this solution if you were using a UID now, and you had a username on the system that matched that UID but itself had a different UID. Not sure why any sane person would do that, but the solution would be to just add '@' onto the front of the UID.

Yes, this seems to be a reasonable approach and it's even listed in the Red Hat link above:

To disambiguate these cases, coreutils supports prefixing user/group identifiers with + to ensure they are interpreted numerically.

@Malvineous
Copy link
Contributor

Malvineous commented May 31, 2020

Oh I didn't get that far in the Red Hat article, sorry about that! If coreutils uses a + prefix already, then it would make sense to follow their example too.

Also, I see chown +0xF does not work, so they are requiring decimal UIDs, just as a point of interest.

@0x2b3bfa0
Copy link
Author

0x2b3bfa0 commented May 31, 2020

Oh I didn't get that far in the Red Hat article, sorry about that! If coreutils uses a + prefix already, then it would make sense to follow their example too.

Yes, that would make sense, and your out-of-the-box thinking was really accurate. 😄

Also, I see chown +0xF does not work, so they are requiring decimal UIDs, just as a point of interest.

I would like to add that chown +0500 interprets the number as decimal 500, while the current systemd implementation would interpret it as octal 500 / decimal 320 because of the leading 0

@0x2b3bfa0
Copy link
Author

0x2b3bfa0 commented May 31, 2020

That would allow you to specify @0 if you wanted a UID for sure, otherwise 0day or 1234 would be chosen as usernames if they existed, falling back to UIDs only if matching usernames couldn't be found (thus giving you backwards compatibility).

By the way, valid_user_group_name only checks that a given string is a valid user / group name, and doesn't pull data from passwd. As this function can be called for non-existing users, identifier fallback might not be an option.

@0x2b3bfa0
Copy link
Author

I've updated #15987 with a proposed fix, but it keeps being passwd-agnostic and doesn't check for existing user names.

@yuwata yuwata closed this as completed in 156a5fd Jun 1, 2020
eworm-de pushed a commit to eworm-de/systemd that referenced this issue Jun 23, 2020
We would parse numbers with base prefixes as user identifiers. For example,
"0x2b3bfa0" would be interpreted as UID==45334432 and "01750" would be
interpreted as UID==1000. This parsing was used also in cases where either a
user/group name or number may be specified. This means that names like
0x2b3bfa0 would be ambiguous: they are a valid user name according to our
documented relaxed rules, but they would also be parsed as numeric uids.

This behaviour is definitely not expected by users, since tools generally only
accept decimal numbers (e.g. id, getent passwd), while other tools only accept
user names and thus will interpret such strings as user names without even
attempting to convert them to numbers (su, ssh). So let's follow suit and only
accept numbers in decimal notation. Effectively this means that we will reject
such strings as a username/uid/groupname/gid where strict mode is used, and try
to look up a user/group with such a name in relaxed mode.

Since the function changed is fairly low-level and fairly widely used, this
affects multiple tools: loginctl show-user/enable-linger/disable-linger foo',
the third argument in sysusers.d, fourth and fifth arguments in tmpfiles.d,
etc.

Fixes systemd#15985.

(cherry picked from commit 156a5fd)
vbatts pushed a commit to kinvolk/systemd that referenced this issue Nov 12, 2020
We would parse numbers with base prefixes as user identifiers. For example,
"0x2b3bfa0" would be interpreted as UID==45334432 and "01750" would be
interpreted as UID==1000. This parsing was used also in cases where either a
user/group name or number may be specified. This means that names like
0x2b3bfa0 would be ambiguous: they are a valid user name according to our
documented relaxed rules, but they would also be parsed as numeric uids.

This behaviour is definitely not expected by users, since tools generally only
accept decimal numbers (e.g. id, getent passwd), while other tools only accept
user names and thus will interpret such strings as user names without even
attempting to convert them to numbers (su, ssh). So let's follow suit and only
accept numbers in decimal notation. Effectively this means that we will reject
such strings as a username/uid/groupname/gid where strict mode is used, and try
to look up a user/group with such a name in relaxed mode.

Since the function changed is fairly low-level and fairly widely used, this
affects multiple tools: loginctl show-user/enable-linger/disable-linger foo',
the third argument in sysusers.d, fourth and fifth arguments in tmpfiles.d,
etc.

Fixes systemd#15985.

(cherry picked from commit 156a5fd)
bluca pushed a commit to bluca/systemd that referenced this issue Jul 9, 2021
We would parse numbers with base prefixes as user identifiers. For example,
"0x2b3bfa0" would be interpreted as UID==45334432 and "01750" would be
interpreted as UID==1000. This parsing was used also in cases where either a
user/group name or number may be specified. This means that names like
0x2b3bfa0 would be ambiguous: they are a valid user name according to our
documented relaxed rules, but they would also be parsed as numeric uids.

This behaviour is definitely not expected by users, since tools generally only
accept decimal numbers (e.g. id, getent passwd), while other tools only accept
user names and thus will interpret such strings as user names without even
attempting to convert them to numbers (su, ssh). So let's follow suit and only
accept numbers in decimal notation. Effectively this means that we will reject
such strings as a username/uid/groupname/gid where strict mode is used, and try
to look up a user/group with such a name in relaxed mode.

Since the function changed is fairly low-level and fairly widely used, this
affects multiple tools: loginctl show-user/enable-linger/disable-linger foo',
the third argument in sysusers.d, fourth and fifth arguments in tmpfiles.d,
etc.

Fixes systemd#15985.

CVE: CVE-2020-13776
Upstream-Status: Backport
[systemd@156a5fd]

Signed-off-by: Dan Tran <dantran@microsoft.com>
russell-islam pushed a commit to russell-islam/systemd that referenced this issue Jul 21, 2021
We would parse numbers with base prefixes as user identifiers. For example,
"0x2b3bfa0" would be interpreted as UID==45334432 and "01750" would be
interpreted as UID==1000. This parsing was used also in cases where either a
user/group name or number may be specified. This means that names like
0x2b3bfa0 would be ambiguous: they are a valid user name according to our
documented relaxed rules, but they would also be parsed as numeric uids.

This behaviour is definitely not expected by users, since tools generally only
accept decimal numbers (e.g. id, getent passwd), while other tools only accept
user names and thus will interpret such strings as user names without even
attempting to convert them to numbers (su, ssh). So let's follow suit and only
accept numbers in decimal notation. Effectively this means that we will reject
such strings as a username/uid/groupname/gid where strict mode is used, and try
to look up a user/group with such a name in relaxed mode.

Since the function changed is fairly low-level and fairly widely used, this
affects multiple tools: loginctl show-user/enable-linger/disable-linger foo',
the third argument in sysusers.d, fourth and fifth arguments in tmpfiles.d,
etc.

Fixes systemd#15985.

CVE: CVE-2020-13776
Upstream-Status: Backport
[systemd@156a5fd]

Signed-off-by: Dan Tran <dantran@microsoft.com>
mikhailnov pushed a commit to mikhailnov/systemd that referenced this issue Aug 10, 2021
We would parse numbers with base prefixes as user identifiers. For example,
"0x2b3bfa0" would be interpreted as UID==45334432 and "01750" would be
interpreted as UID==1000. This parsing was used also in cases where either a
user/group name or number may be specified. This means that names like
0x2b3bfa0 would be ambiguous: they are a valid user name according to our
documented relaxed rules, but they would also be parsed as numeric uids.

This behaviour is definitely not expected by users, since tools generally only
accept decimal numbers (e.g. id, getent passwd), while other tools only accept
user names and thus will interpret such strings as user names without even
attempting to convert them to numbers (su, ssh). So let's follow suit and only
accept numbers in decimal notation. Effectively this means that we will reject
such strings as a username/uid/groupname/gid where strict mode is used, and try
to look up a user/group with such a name in relaxed mode.

Since the function changed is fairly low-level and fairly widely used, this
affects multiple tools: loginctl show-user/enable-linger/disable-linger foo',
the third argument in sysusers.d, fourth and fifth arguments in tmpfiles.d,
etc.

Fixes systemd#15985.
mikhailnov pushed a commit to mikhailnov/systemd that referenced this issue Aug 10, 2021
We would parse numbers with base prefixes as user identifiers. For example,
"0x2b3bfa0" would be interpreted as UID==45334432 and "01750" would be
interpreted as UID==1000. This parsing was used also in cases where either a
user/group name or number may be specified. This means that names like
0x2b3bfa0 would be ambiguous: they are a valid user name according to our
documented relaxed rules, but they would also be parsed as numeric uids.

This behaviour is definitely not expected by users, since tools generally only
accept decimal numbers (e.g. id, getent passwd), while other tools only accept
user names and thus will interpret such strings as user names without even
attempting to convert them to numbers (su, ssh). So let's follow suit and only
accept numbers in decimal notation. Effectively this means that we will reject
such strings as a username/uid/groupname/gid where strict mode is used, and try
to look up a user/group with such a name in relaxed mode.

Since the function changed is fairly low-level and fairly widely used, this
affects multiple tools: loginctl show-user/enable-linger/disable-linger foo',
the third argument in sysusers.d, fourth and fifth arguments in tmpfiles.d,
etc.

Fixes systemd#15985.
mikhailnov pushed a commit to mikhailnov/systemd that referenced this issue Aug 10, 2021
We would parse numbers with base prefixes as user identifiers. For example,
"0x2b3bfa0" would be interpreted as UID==45334432 and "01750" would be
interpreted as UID==1000. This parsing was used also in cases where either a
user/group name or number may be specified. This means that names like
0x2b3bfa0 would be ambiguous: they are a valid user name according to our
documented relaxed rules, but they would also be parsed as numeric uids.

This behaviour is definitely not expected by users, since tools generally only
accept decimal numbers (e.g. id, getent passwd), while other tools only accept
user names and thus will interpret such strings as user names without even
attempting to convert them to numbers (su, ssh). So let's follow suit and only
accept numbers in decimal notation. Effectively this means that we will reject
such strings as a username/uid/groupname/gid where strict mode is used, and try
to look up a user/group with such a name in relaxed mode.

Since the function changed is fairly low-level and fairly widely used, this
affects multiple tools: loginctl show-user/enable-linger/disable-linger foo',
the third argument in sysusers.d, fourth and fifth arguments in tmpfiles.d,
etc.

Fixes systemd#15985.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants