From: Lennart Poettering <lennart@poettering.net>
Date: Mon, 1 Jun 2020 15:16:46 +0000 (+0200)
Subject: user-util: be stricter in parse_uid()
X-Git-Tag: v246-rc1~202^2~4
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=f5979b63cc305ba217dfd174b1bf0583bcf75a73;p=thirdparty%2Fsystemd.git

user-util: be stricter in parse_uid()

Let's refuse "+" and "-" prefixed UIDs. Let's refuse whitespace-prefixed
UIDS, Let's refuse zero-prefixed UIDs. Let's be safe than sorry.
---

diff --git a/src/basic/user-util.c b/src/basic/user-util.c
index 2db8ef6abf7..4d087b1d3e3 100644
--- a/src/basic/user-util.c
+++ b/src/basic/user-util.c
@@ -49,7 +49,15 @@ int parse_uid(const char *s, uid_t *ret) {
         assert(s);
 
         assert_cc(sizeof(uid_t) == sizeof(uint32_t));
-        r = safe_atou32_full(s, 10, &uid);
+
+        /* We are very strict when parsing UIDs, and prohibit +/- as prefix, leading zero as prefix, and
+         * whitespace. We do this, since this call is often used in a context where we parse things as UID
+         * first, and if that doesn't work we fall back to NSS. Thus we really want to make sure that UIDs
+         * are parsed as UIDs only if they really really look like UIDs. */
+        r = safe_atou32_full(s, 10
+                             | SAFE_ATO_REFUSE_PLUS_MINUS
+                             | SAFE_ATO_REFUSE_LEADING_ZERO
+                             | SAFE_ATO_REFUSE_LEADING_WHITESPACE, &uid);
         if (r < 0)
                 return r;
 
diff --git a/src/test/test-user-util.c b/src/test/test-user-util.c
index 3165232fef6..7d2efc8c599 100644
--- a/src/test/test-user-util.c
+++ b/src/test/test-user-util.c
@@ -54,13 +54,33 @@ static void test_parse_uid(void) {
         assert_se(r == -EINVAL);
         assert_se(uid == 100);
 
+        r = parse_uid("+1234", &uid);
+        assert_se(r == -EINVAL);
+        assert_se(uid == 100);
+
+        r = parse_uid("-1234", &uid);
+        assert_se(r == -EINVAL);
+        assert_se(uid == 100);
+
+        r = parse_uid(" 1234", &uid);
+        assert_se(r == -EINVAL);
+        assert_se(uid == 100);
+
         r = parse_uid("01234", &uid);
-        assert_se(r == 0);
-        assert_se(uid == 1234);
+        assert_se(r == -EINVAL);
+        assert_se(uid == 100);
+
+        r = parse_uid("-0", &uid);
+        assert_se(r == -EINVAL);
+        assert_se(uid == 100);
+
+        r = parse_uid("+0", &uid);
+        assert_se(r == -EINVAL);
+        assert_se(uid == 100);
 
         r = parse_uid("asdsdas", &uid);
         assert_se(r == -EINVAL);
-        assert_se(uid == 1234);
+        assert_se(uid == 100);
 }
 
 static void test_uid_ptr(void) {