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] == '-') + 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; - 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) - 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; }