Merge pull request #16033 from poettering/parse-int-fixlets

various fixes and tweaks for integer parsing

5 files changed, 316 insertions, 47 deletions

diff --git a/src/basic/parse-util.c b/src/basic/parse-util.c
index 59f8a31cec..44f0438cf4 100644
--- a/src/basic/parse-util.c
+++ b/src/basic/parse-util.c
@@ -70,26 +70,24 @@ int parse_pid(const char *s, pid_t* ret_pid) {
 }
 
 int parse_mode(const char *s, mode_t *ret) {
-        char *x;
-        long l;
+        unsigned m;
+        int r;
 
         assert(s);
-        assert(ret);
-
-        s += strspn(s, WHITESPACE);
-        if (s[0] == '-')
-                return -ERANGE;
 
-        errno = 0;
-        l = strtol(s, &x, 8);
-        if (errno > 0)
-                return -errno;
-        if (!x || x == s || *x != 0)
-                return -EINVAL;
-        if (l < 0 || l  > 07777)
+        r = safe_atou_full(s, 8 |
+                           SAFE_ATO_REFUSE_PLUS_MINUS, /* Leading '+' or even '-' char? that's just weird,
+                                                        * refuse. User might have wanted to add mode flags or
+                                                        * so, but this parser doesn't allow that, so let's
+                                                        * better be safe. */
+                           &m);
+        if (r < 0)
+                return r;
+        if (m > 07777)
                 return -ERANGE;
 
-        *ret = (mode_t) l;
+        if (ret)
+                *ret = m;
         return 0;
 }
 
@@ -354,30 +352,73 @@ int parse_syscall_and_errno(const char *in, char **name, int *error) {
         return 0;
 }
 
+static const char *mangle_base(const char *s, unsigned *base) {
+        const char *k;
+
+        assert(s);
+        assert(base);
+
+        /* Base already explicitly specified, then don't do anything. */
+        if (SAFE_ATO_MASK_FLAGS(*base) != 0)
+                return s;
+
+        /* Support Python 3 style "0b" and 0x" prefixes, because they truly make sense, much more than C's "0" prefix for octal. */
+        k = STARTSWITH_SET(s, "0b", "0B");
+        if (k) {
+                *base = 2 | (*base & SAFE_ATO_ALL_FLAGS);
+                return k;
+        }
+
+        k = STARTSWITH_SET(s, "0o", "0O");
+        if (k) {
+                *base = 8 | (*base & SAFE_ATO_ALL_FLAGS);
+                return k;
+        }
+
+        return s;
+}
+
 int safe_atou_full(const char *s, unsigned base, unsigned *ret_u) {
         char *x = NULL;
         unsigned long l;
 
         assert(s);
-        assert(base <= 16);
+        assert(SAFE_ATO_MASK_FLAGS(base) <= 16);
 
-        /* strtoul() is happy to parse negative values, and silently
-         * converts them to unsigned values without generating an
-         * error. We want a clean error, hence let's look for the "-"
-         * prefix on our own, and generate an error. But let's do so
-         * only after strtoul() validated that the string is clean
-         * otherwise, so that we return EINVAL preferably over
-         * ERANGE. */
+        /* strtoul() is happy to parse negative values, and silently converts them to unsigned values without
+         * generating an error. We want a clean error, hence let's look for the "-" prefix on our own, and
+         * generate an error. But let's do so only after strtoul() validated that the string is clean
+         * otherwise, so that we return EINVAL preferably over ERANGE. */
+
+        if (FLAGS_SET(base, SAFE_ATO_REFUSE_LEADING_WHITESPACE) &&
+            strchr(WHITESPACE, s[0]))
+                return -EINVAL;
 
         s += strspn(s, WHITESPACE);
 
+        if (FLAGS_SET(base, SAFE_ATO_REFUSE_PLUS_MINUS) &&
+            IN_SET(s[0], '+', '-'))
+                return -EINVAL; /* Note that we check the "-" prefix again a second time below, but return a
+                                 * different error. I.e. if the SAFE_ATO_REFUSE_PLUS_MINUS flag is set we
+                                 * blanket refuse +/- prefixed integers, while if it is missing we'll just
+                                 * return ERANGE, because the string actually parses correctly, but doesn't
+                                 * fit in the return type. */
+
+        if (FLAGS_SET(base, SAFE_ATO_REFUSE_LEADING_ZERO) &&
+            s[0] == '0' && !streq(s, "0"))
+                return -EINVAL; /* This is particularly useful to avoid ambiguities between C's octal
+                                 * notation and assumed-to-be-decimal integers with a leading zero. */
+
+        s = mangle_base(s, &base);
+
         errno = 0;
-        l = strtoul(s, &x, base);
+        l = strtoul(s, &x, SAFE_ATO_MASK_FLAGS(base) /* Let's mask off the flags bits so that only the actual
+                                                      * base is left */);
         if (errno > 0)
                 return -errno;
         if (!x || x == s || *x != 0)
                 return -EINVAL;
-        if (s[0] == '-')
+        if (l != 0 && s[0] == '-')
                 return -ERANGE;
         if ((unsigned long) (unsigned) l != l)
                 return -ERANGE;
@@ -389,13 +430,17 @@ int safe_atou_full(const char *s, unsigned base, unsigned *ret_u) {
 }
 
 int safe_atoi(const char *s, int *ret_i) {
+        unsigned base = 0;
         char *x = NULL;
         long l;
 
         assert(s);
 
+        s += strspn(s, WHITESPACE);
+        s = mangle_base(s, &base);
+
         errno = 0;
-        l = strtol(s, &x, 0);
+        l = strtol(s, &x, base);
         if (errno > 0)
                 return -errno;
         if (!x || x == s || *x != 0)
@@ -414,16 +459,31 @@ int safe_atollu_full(const char *s, unsigned base, long long unsigned *ret_llu)
         unsigned long long l;
 
         assert(s);
+        assert(SAFE_ATO_MASK_FLAGS(base) <= 16);
+
+        if (FLAGS_SET(base, SAFE_ATO_REFUSE_LEADING_WHITESPACE) &&
+            strchr(WHITESPACE, s[0]))
+                return -EINVAL;
 
         s += strspn(s, WHITESPACE);
 
+        if (FLAGS_SET(base, SAFE_ATO_REFUSE_PLUS_MINUS) &&
+            IN_SET(s[0], '+', '-'))
+                return -EINVAL;
+
+        if (FLAGS_SET(base, SAFE_ATO_REFUSE_LEADING_ZERO) &&
+            s[0] == '0' && s[1] != 0)
+                return -EINVAL;
+
+        s = mangle_base(s, &base);
+
         errno = 0;
-        l = strtoull(s, &x, base);
+        l = strtoull(s, &x, SAFE_ATO_MASK_FLAGS(base));
         if (errno > 0)
                 return -errno;
         if (!x || x == s || *x != 0)
                 return -EINVAL;
-        if (*s == '-')
+        if (l != 0 && s[0] == '-')
                 return -ERANGE;
 
         if (ret_llu)
@@ -433,13 +493,17 @@ int safe_atollu_full(const char *s, unsigned base, long long unsigned *ret_llu)
 }
 
 int safe_atolli(const char *s, long long int *ret_lli) {
+        unsigned base = 0;
         char *x = NULL;
         long long l;
 
         assert(s);
 
+        s += strspn(s, WHITESPACE);
+        s = mangle_base(s, &base);
+
         errno = 0;
-        l = strtoll(s, &x, 0);
+        l = strtoll(s, &x, base);
         if (errno > 0)
                 return -errno;
         if (!x || x == s || *x != 0)
@@ -452,20 +516,22 @@ int safe_atolli(const char *s, long long int *ret_lli) {
 }
 
 int safe_atou8(const char *s, uint8_t *ret) {
-        char *x = NULL;
+        unsigned base = 0;
         unsigned long l;
+        char *x = NULL;
 
         assert(s);
 
         s += strspn(s, WHITESPACE);
+        s = mangle_base(s, &base);
 
         errno = 0;
-        l = strtoul(s, &x, 0);
+        l = strtoul(s, &x, base);
         if (errno > 0)
                 return -errno;
         if (!x || x == s || *x != 0)
                 return -EINVAL;
-        if (s[0] == '-')
+        if (l != 0 && s[0] == '-')
                 return -ERANGE;
         if ((unsigned long) (uint8_t) l != l)
                 return -ERANGE;
@@ -480,34 +546,53 @@ int safe_atou16_full(const char *s, unsigned base, uint16_t *ret) {
         unsigned long l;
 
         assert(s);
-        assert(ret);
-        assert(base <= 16);
+        assert(SAFE_ATO_MASK_FLAGS(base) <= 16);
+
+        if (FLAGS_SET(base, SAFE_ATO_REFUSE_LEADING_WHITESPACE) &&
+            strchr(WHITESPACE, s[0]))
+                return -EINVAL;
 
         s += strspn(s, WHITESPACE);
 
+        if (FLAGS_SET(base, SAFE_ATO_REFUSE_PLUS_MINUS) &&
+            IN_SET(s[0], '+', '-'))
+                return -EINVAL;
+
+        if (FLAGS_SET(base, SAFE_ATO_REFUSE_LEADING_ZERO) &&
+            s[0] == '0' && s[1] != 0)
+                return -EINVAL;
+
+        s = mangle_base(s, &base);
+
         errno = 0;
-        l = strtoul(s, &x, base);
+        l = strtoul(s, &x, SAFE_ATO_MASK_FLAGS(base));
         if (errno > 0)
                 return -errno;
         if (!x || x == s || *x != 0)
                 return -EINVAL;
-        if (s[0] == '-')
+        if (l != 0 && s[0] == '-')
                 return -ERANGE;
         if ((unsigned long) (uint16_t) l != l)
                 return -ERANGE;
 
-        *ret = (uint16_t) l;
+        if (ret)
+                *ret = (uint16_t) l;
+
         return 0;
 }
 
 int safe_atoi16(const char *s, int16_t *ret) {
+        unsigned base = 0;
         char *x = NULL;
         long l;
 
         assert(s);
 
+        s += strspn(s, WHITESPACE);
+        s = mangle_base(s, &base);
+
         errno = 0;
-        l = strtol(s, &x, 0);
+        l = strtol(s, &x, base);
         if (errno > 0)
                 return -errno;
         if (!x || x == s || *x != 0)
diff --git a/src/basic/parse-util.h b/src/basic/parse-util.h
index 970bdefbf0..9a516ce5f6 100644
--- a/src/basic/parse-util.h
+++ b/src/basic/parse-util.h
@@ -21,6 +21,12 @@ int parse_range(const char *t, unsigned *lower, unsigned *upper);
 int parse_errno(const char *t);
 int parse_syscall_and_errno(const char *in, char **name, int *error);
 
+#define SAFE_ATO_REFUSE_PLUS_MINUS (1U << 30)
+#define SAFE_ATO_REFUSE_LEADING_ZERO (1U << 29)
+#define SAFE_ATO_REFUSE_LEADING_WHITESPACE (1U << 28)
+#define SAFE_ATO_ALL_FLAGS (SAFE_ATO_REFUSE_PLUS_MINUS|SAFE_ATO_REFUSE_LEADING_ZERO|SAFE_ATO_REFUSE_LEADING_WHITESPACE)
+#define SAFE_ATO_MASK_FLAGS(base) ((base) & ~SAFE_ATO_ALL_FLAGS)
+
 int safe_atou_full(const char *s, unsigned base, unsigned *ret_u);
 
 static inline int safe_atou(const char *s, unsigned *ret_u) {
diff --git a/src/basic/user-util.c b/src/basic/user-util.c
index 2db8ef6abf..7dd2f6664a 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;
 
@@ -66,22 +74,39 @@ int parse_uid(const char *s, uid_t *ret) {
 }
 
 int parse_uid_range(const char *s, uid_t *ret_lower, uid_t *ret_upper) {
-        uint32_t u, l;
+        _cleanup_free_ char *word = NULL;
+        uid_t l, u;
         int r;
 
         assert(s);
         assert(ret_lower);
         assert(ret_upper);
 
-        r = parse_range(s, &l, &u);
+        r = extract_first_word(&s, &word, "-", EXTRACT_DONT_COALESCE_SEPARATORS);
+        if (r < 0)
+                return r;
+        if (r == 0)
+                return -EINVAL;
+
+        r = parse_uid(word, &l);
         if (r < 0)
                 return r;
 
-        if (l > u)
+        /* Check for the upper bound and extract it if needed */
+        if (!s)
+                /* Single number with no dash. */
+                u = l;
+        else if (!*s)
+                /* Trailing dash is an error. */
                 return -EINVAL;
+        else {
+                r = parse_uid(s, &u);
+                if (r < 0)
+                        return r;
 
-        if (!uid_is_valid(l) || !uid_is_valid(u))
-                return -ENXIO;
+                if (l > u)
+                        return -EINVAL;
+        }
 
         *ret_lower = l;
         *ret_upper = u;
diff --git a/src/test/test-parse-util.c b/src/test/test-parse-util.c
index 1627bc747d..3ca5e1e639 100644
--- a/src/test/test-parse-util.c
+++ b/src/test/test-parse-util.c
@@ -75,14 +75,22 @@ static void test_parse_mode(void) {
         mode_t m;
 
         assert_se(parse_mode("-1", &m) < 0);
+        assert_se(parse_mode("+1", &m) < 0);
         assert_se(parse_mode("", &m) < 0);
         assert_se(parse_mode("888", &m) < 0);
         assert_se(parse_mode("77777", &m) < 0);
 
         assert_se(parse_mode("544", &m) >= 0 && m == 0544);
+        assert_se(parse_mode("0544", &m) >= 0 && m == 0544);
+        assert_se(parse_mode("00544", &m) >= 0 && m == 0544);
         assert_se(parse_mode("777", &m) >= 0 && m == 0777);
+        assert_se(parse_mode("0777", &m) >= 0 && m == 0777);
+        assert_se(parse_mode("00777", &m) >= 0 && m == 0777);
         assert_se(parse_mode("7777", &m) >= 0 && m == 07777);
+        assert_se(parse_mode("07777", &m) >= 0 && m == 07777);
+        assert_se(parse_mode("007777", &m) >= 0 && m == 07777);
         assert_se(parse_mode("0", &m) >= 0 && m == 0);
+        assert_se(parse_mode(" 1", &m) >= 0 && m == 1);
 }
 
 static void test_parse_size(void) {
@@ -358,6 +366,18 @@ static void test_safe_atolli(void) {
         assert_se(r == 0);
         assert_se(l == -12345);
 
+        r = safe_atolli("0x5", &l);
+        assert_se(r == 0);
+        assert_se(l == 5);
+
+        r = safe_atolli("0o6", &l);
+        assert_se(r == 0);
+        assert_se(l == 6);
+
+        r = safe_atolli("0B101", &l);
+        assert_se(r == 0);
+        assert_se(l == 5);
+
         r = safe_atolli("12345678901234567890", &l);
         assert_se(r == -ERANGE);
 
@@ -431,6 +451,14 @@ static void test_safe_atoi16(void) {
         assert_se(r == 0);
         assert_se(l == 32767);
 
+        r = safe_atoi16("0o11", &l);
+        assert_se(r == 0);
+        assert_se(l == 9);
+
+        r = safe_atoi16("0B110", &l);
+        assert_se(r == 0);
+        assert_se(l == 6);
+
         r = safe_atoi16("36536", &l);
         assert_se(r == -ERANGE);
 
@@ -475,6 +503,13 @@ static void test_safe_atoux16(void) {
         r = safe_atoux16("  -1", &l);
         assert_se(r == -ERANGE);
 
+        r = safe_atoux16("0b1", &l);
+        assert_se(r == 0);
+        assert_se(l == 177);
+
+        r = safe_atoux16("0o70", &l);
+        assert_se(r == -EINVAL);
+
         r = safe_atoux16("junk", &l);
         assert_se(r == -EINVAL);
 
@@ -500,6 +535,14 @@ static void test_safe_atou64(void) {
         assert_se(r == 0);
         assert_se(l == 12345);
 
+        r = safe_atou64("0o11", &l);
+        assert_se(r == 0);
+        assert_se(l == 9);
+
+        r = safe_atou64("0b11", &l);
+        assert_se(r == 0);
+        assert_se(l == 3);
+
         r = safe_atou64("18446744073709551617", &l);
         assert_se(r == -ERANGE);
 
@@ -542,6 +585,14 @@ static void test_safe_atoi64(void) {
         assert_se(r == 0);
         assert_se(l == 32767);
 
+        r = safe_atoi64("  0o20", &l);
+        assert_se(r == 0);
+        assert_se(l == 16);
+
+        r = safe_atoi64("  0b01010", &l);
+        assert_se(r == 0);
+        assert_se(l == 10);
+
         r = safe_atoi64("9223372036854775813", &l);
         assert_se(r == -ERANGE);
 
@@ -577,6 +628,13 @@ static void test_safe_atoux64(void) {
         assert_se(r == 0);
         assert_se(l == 0x12345);
 
+        r = safe_atoux64("0b11011", &l);
+        assert_se(r == 0);
+        assert_se(l == 11603985);
+
+        r = safe_atoux64("0o11011", &l);
+        assert_se(r == -EINVAL);
+
         r = safe_atoux64("18446744073709551617", &l);
         assert_se(r == -ERANGE);
 
diff --git a/src/test/test-user-util.c b/src/test/test-user-util.c
index 3165232fef..c9bff941be 100644
--- a/src/test/test-user-util.c
+++ b/src/test/test-user-util.c
@@ -42,6 +42,22 @@ static void test_parse_uid(void) {
 
         log_info("/* %s */", __func__);
 
+        r = parse_uid("0", &uid);
+        assert_se(r == 0);
+        assert_se(uid == 0);
+
+        r = parse_uid("1", &uid);
+        assert_se(r == 0);
+        assert_se(uid == 1);
+
+        r = parse_uid("01", &uid);
+        assert_se(r == -EINVAL);
+        assert_se(uid == 1);
+
+        r = parse_uid("001", &uid);
+        assert_se(r == -EINVAL);
+        assert_se(uid == 1);
+
         r = parse_uid("100", &uid);
         assert_se(r == 0);
         assert_se(uid == 100);
@@ -54,13 +70,57 @@ static void test_parse_uid(void) {
         assert_se(r == -EINVAL);
         assert_se(uid == 100);
 
+        r = parse_uid("0o1234", &uid);
+        assert_se(r == -EINVAL);
+        assert_se(uid == 100);
+
+        r = parse_uid("0b1234", &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(" 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("001234", &uid);
+        assert_se(r == -EINVAL);
+        assert_se(uid == 100);
+
+        r = parse_uid("0001234", &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("+0", &uid);
+        assert_se(r == -EINVAL);
+        assert_se(uid == 100);
+
+        r = parse_uid("00", &uid);
+        assert_se(r == -EINVAL);
+        assert_se(uid == 100);
+
+        r = parse_uid("000", &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) {
@@ -359,6 +419,39 @@ static void test_gid_lists_ops(void) {
         assert_se(gids);
 }
 
+static void test_parse_uid_range(void) {
+        uid_t a = 4711, b = 4711;
+
+        log_info("/* %s */", __func__);
+
+        assert_se(parse_uid_range("", &a, &b) == -EINVAL && a == 4711 && b == 4711);
+        assert_se(parse_uid_range(" ", &a, &b) == -EINVAL && a == 4711 && b == 4711);
+        assert_se(parse_uid_range("x", &a, &b) == -EINVAL && a == 4711 && b == 4711);
+
+        assert_se(parse_uid_range("0", &a, &b) >= 0 && a == 0 && b == 0);
+        assert_se(parse_uid_range("1", &a, &b) >= 0 && a == 1 && b == 1);
+        assert_se(parse_uid_range("2-2", &a, &b) >= 0 && a == 2 && b == 2);
+        assert_se(parse_uid_range("3-3", &a, &b) >= 0 && a == 3 && b == 3);
+        assert_se(parse_uid_range("4-5", &a, &b) >= 0 && a == 4 && b == 5);
+
+        assert_se(parse_uid_range("7-6", &a, &b) == -EINVAL && a == 4 && b == 5);
+        assert_se(parse_uid_range("-1", &a, &b) == -EINVAL && a == 4 && b == 5);
+        assert_se(parse_uid_range("01", &a, &b) == -EINVAL && a == 4 && b == 5);
+        assert_se(parse_uid_range("001", &a, &b) == -EINVAL && a == 4 && b == 5);
+        assert_se(parse_uid_range("+1", &a, &b) == -EINVAL && a == 4 && b == 5);
+        assert_se(parse_uid_range("1--1", &a, &b) == -EINVAL && a == 4 && b == 5);
+        assert_se(parse_uid_range(" 1", &a, &b) == -EINVAL && a == 4 && b == 5);
+        assert_se(parse_uid_range(" 1-2", &a, &b) == -EINVAL && a == 4 && b == 5);
+        assert_se(parse_uid_range("1 -2", &a, &b) == -EINVAL && a == 4 && b == 5);
+        assert_se(parse_uid_range("1- 2", &a, &b) == -EINVAL && a == 4 && b == 5);
+        assert_se(parse_uid_range("1-2 ", &a, &b) == -EINVAL && a == 4 && b == 5);
+        assert_se(parse_uid_range("01-2", &a, &b) == -EINVAL && a == 4 && b == 5);
+        assert_se(parse_uid_range("1-02", &a, &b) == -EINVAL && a == 4 && b == 5);
+        assert_se(parse_uid_range("001-2", &a, &b) == -EINVAL && a == 4 && b == 5);
+        assert_se(parse_uid_range("1-002", &a, &b) == -EINVAL && a == 4 && b == 5);
+        assert_se(parse_uid_range(" 01", &a, &b) == -EINVAL && a == 4 && b == 5);
+}
+
 int main(int argc, char *argv[]) {
         test_uid_to_name_one(0, "root");
         test_uid_to_name_one(UID_NOBODY, NOBODY_USER_NAME);
@@ -396,5 +489,7 @@ int main(int argc, char *argv[]) {
         test_in_gid();
         test_gid_lists_ops();
 
+        test_parse_uid_range();
+
         return 0;
 }