From: Lennart Poettering Date: Sun, 3 Dec 2017 19:57:24 +0000 (+0100) Subject: util-lib,tests: rework unbase64 so that we skip over whitespace automatically (#7522) X-Git-Tag: v236~75 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=081f36d82de848ab93acbc19cac0d79186e9321c;p=thirdparty%2Fsystemd.git util-lib,tests: rework unbase64 so that we skip over whitespace automatically (#7522) Let's optimize things a bit, and instead of having to strip whitespace first before decoding base64, let's do that implicitly while doing so. Given that base64 was designed the way it was designed specifically to be tolerant to whitespace changes, it's a good idea to do this automatically and implicitly. --- diff --git a/src/basic/hexdecoct.c b/src/basic/hexdecoct.c index 3ffedcafe5a..e080aaa175c 100644 --- a/src/basic/hexdecoct.c +++ b/src/basic/hexdecoct.c @@ -624,112 +624,135 @@ int base64_append( return base64_append_width(prefix, plen, NULL, plen, p, l, width - plen - 1); } -int unbase64mem(const char *p, size_t l, void **mem, size_t *_len) { - _cleanup_free_ uint8_t *r = NULL; - int a, b, c, d; - uint8_t *z; +static int unbase64_next(const char **p, size_t *l) { + int ret; + + assert(p); + assert(l); + + /* Find the next non-whitespace character, and decode it. If we find padding, we return it as INT_MAX. We + * greedily skip all preceeding and all following whitespace. */ + + for (;;) { + if (*l == 0) + return -EPIPE; + + if (!strchr(WHITESPACE, **p)) + break; + + /* Skip leading whitespace */ + (*p)++, (*l)--; + } + + if (**p == '=') + ret = INT_MAX; /* return padding as INT_MAX */ + else { + ret = unbase64char(**p); + if (ret < 0) + return ret; + } + + for (;;) { + (*p)++, (*l)--; + + if (*l == 0) + break; + if (!strchr(WHITESPACE, **p)) + break; + + /* Skip following whitespace */ + } + + return ret; +} + +int unbase64mem(const char *p, size_t l, void **ret, size_t *ret_size) { + _cleanup_free_ uint8_t *buf = NULL; const char *x; + uint8_t *z; size_t len; assert(p || l == 0); - assert(mem); - assert(_len); + assert(ret); + assert(ret_size); if (l == (size_t) -1) l = strlen(p); - /* padding ensures any base63 input has input divisible by 4 */ - if (l % 4 != 0) - return -EINVAL; - - /* strip the padding */ - if (l > 0 && p[l - 1] == '=') - l--; - if (l > 0 && p[l - 1] == '=') - l--; - - /* a group of four input bytes needs three output bytes, in case of - padding we need to add two or three extra bytes */ - len = (l / 4) * 3 + (l % 4 ? (l % 4) - 1 : 0); + /* A group of four input bytes needs three output bytes, in case of padding we need to add two or three extra + bytes. Note that this calculation is an upper boundary, as we ignore whitespace while decoding */ + len = (l / 4) * 3 + (l % 4 != 0 ? (l % 4) - 1 : 0); - z = r = malloc(len + 1); - if (!r) + buf = malloc(len + 1); + if (!buf) return -ENOMEM; - for (x = p; x < p + (l / 4) * 4; x += 4) { - /* a == 00XXXXXX; b == 00YYYYYY; c == 00ZZZZZZ; d == 00WWWWWW */ - a = unbase64char(x[0]); + for (x = p, z = buf;;) { + int a, b, c, d; /* a == 00XXXXXX; b == 00YYYYYY; c == 00ZZZZZZ; d == 00WWWWWW */ + + a = unbase64_next(&x, &l); + if (a == -EPIPE) /* End of string */ + break; if (a < 0) + return a; + if (a == INT_MAX) /* Padding is not allowed at at the beginning of a 4ch block */ return -EINVAL; - b = unbase64char(x[1]); + b = unbase64_next(&x, &l); if (b < 0) + return b; + if (b == INT_MAX) /* Padding is not allowed at the second character of a 4ch block either */ return -EINVAL; - c = unbase64char(x[2]); + c = unbase64_next(&x, &l); if (c < 0) - return -EINVAL; + return c; - d = unbase64char(x[3]); + d = unbase64_next(&x, &l); if (d < 0) - return -EINVAL; - - *(z++) = (uint8_t) a << 2 | (uint8_t) b >> 4; /* XXXXXXYY */ - *(z++) = (uint8_t) b << 4 | (uint8_t) c >> 2; /* YYYYZZZZ */ - *(z++) = (uint8_t) c << 6 | (uint8_t) d; /* ZZWWWWWW */ - } - - switch (l % 4) { + return d; - case 3: - a = unbase64char(x[0]); - if (a < 0) - return -EINVAL; + if (c == INT_MAX) { /* Padding at the third character */ - b = unbase64char(x[1]); - if (b < 0) - return -EINVAL; + if (d != INT_MAX) /* If the third character is padding, the fourth must be too */ + return -EINVAL; - c = unbase64char(x[2]); - if (c < 0) - return -EINVAL; + /* b == 00YY0000 */ + if (b & 15) + return -EINVAL; - /* c == 00ZZZZ00 */ - if (c & 3) - return -EINVAL; - - *(z++) = (uint8_t) a << 2 | (uint8_t) b >> 4; /* XXXXXXYY */ - *(z++) = (uint8_t) b << 4 | (uint8_t) c >> 2; /* YYYYZZZZ */ - - break; - case 2: - a = unbase64char(x[0]); - if (a < 0) - return -EINVAL; + if (l > 0) /* Trailing rubbish? */ + return -ENAMETOOLONG; - b = unbase64char(x[1]); - if (b < 0) - return -EINVAL; + *(z++) = (uint8_t) a << 2 | (uint8_t) (b >> 4); /* XXXXXXYY */ + break; + } - /* b == 00YY0000 */ - if (b & 15) - return -EINVAL; + if (d == INT_MAX) { + /* c == 00ZZZZ00 */ + if (c & 3) + return -EINVAL; - *(z++) = (uint8_t) a << 2 | (uint8_t) (b >> 4); /* XXXXXXYY */ + if (l > 0) /* Trailing rubbish? */ + return -ENAMETOOLONG; - break; - case 0: + *(z++) = (uint8_t) a << 2 | (uint8_t) b >> 4; /* XXXXXXYY */ + *(z++) = (uint8_t) b << 4 | (uint8_t) c >> 2; /* YYYYZZZZ */ + break; + } - break; - default: - return -EINVAL; + *(z++) = (uint8_t) a << 2 | (uint8_t) b >> 4; /* XXXXXXYY */ + *(z++) = (uint8_t) b << 4 | (uint8_t) c >> 2; /* YYYYZZZZ */ + *(z++) = (uint8_t) c << 6 | (uint8_t) d; /* ZZWWWWWW */ } *z = 0; - *mem = r; - r = NULL; - *_len = len; + if (ret_size) + *ret_size = (size_t) (z - buf); + + *ret = buf; + buf = NULL; return 0; } diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 035fa8844bc..d99176adbe0 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -978,7 +978,6 @@ int config_parse_exec_input_data( void *data, void *userdata) { - _cleanup_free_ char *cleaned = NULL; _cleanup_free_ void *p = NULL; ExecContext *c = data; size_t sz; @@ -997,15 +996,9 @@ int config_parse_exec_input_data( return 0; } - /* Be tolerant to whitespace. Remove it all before decoding */ - cleaned = strdup(rvalue); - if (!cleaned) - return log_oom(); - delete_chars(cleaned, WHITESPACE); - - r = unbase64mem(cleaned, (size_t) -1, &p, &sz); + r = unbase64mem(rvalue, (size_t) -1, &p, &sz); if (r < 0) - return log_syntax(unit, LOG_ERR, filename, line, r, "Failed to decode base64 data, ignoring: %s", cleaned); + return log_syntax(unit, LOG_ERR, filename, line, r, "Failed to decode base64 data, ignoring: %s", rvalue); assert(sz > 0); diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index cfd4c833b0b..6ed00859537 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -415,19 +415,12 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen r = sd_bus_message_append(m, "v", "s", eq); else if (streq(field, "StandardInputData")) { - _cleanup_free_ char *cleaned = NULL; _cleanup_free_ void *decoded = NULL; size_t sz; - cleaned = strdup(eq); - if (!cleaned) - return log_oom(); - - delete_chars(cleaned, WHITESPACE); - - r = unbase64mem(cleaned, (size_t) -1, &decoded, &sz); + r = unbase64mem(eq, (size_t) -1, &decoded, &sz); if (r < 0) - return log_error_errno(r, "Failed to decode base64 data '%s': %m", cleaned); + return log_error_errno(r, "Failed to decode base64 data '%s': %m", eq); r = sd_bus_message_open_container(m, 'v', "ay"); if (r < 0) diff --git a/src/test/test-hexdecoct.c b/src/test/test-hexdecoct.c index 5767baebb13..7675d0cba72 100644 --- a/src/test/test-hexdecoct.c +++ b/src/test/test-hexdecoct.c @@ -301,42 +301,43 @@ static void test_base64mem(void) { free(b64); } -static void test_unbase64mem(void) { - void *mem; - size_t len; - - assert_se(unbase64mem("", strlen(""), &mem, &len) == 0); - assert_se(streq(strndupa(mem, len), "")); - free(mem); +static void test_unbase64mem_one(const char *input, const char *output, int ret) { + _cleanup_free_ void *buffer = NULL; + size_t size = 0; - assert_se(unbase64mem("Zg==", strlen("Zg=="), &mem, &len) == 0); - assert_se(streq(strndupa(mem, len), "f")); - free(mem); - - assert_se(unbase64mem("Zm8=", strlen("Zm8="), &mem, &len) == 0); - assert_se(streq(strndupa(mem, len), "fo")); - free(mem); + assert_se(unbase64mem(input, (size_t) -1, &buffer, &size) == ret); - assert_se(unbase64mem("Zm9v", strlen("Zm9v"), &mem, &len) == 0); - assert_se(streq(strndupa(mem, len), "foo")); - free(mem); - - assert_se(unbase64mem("Zm9vYg==", strlen("Zm9vYg=="), &mem, &len) == 0); - assert_se(streq(strndupa(mem, len), "foob")); - free(mem); - - assert_se(unbase64mem("Zm9vYmE=", strlen("Zm9vYmE="), &mem, &len) == 0); - assert_se(streq(strndupa(mem, len), "fooba")); - free(mem); + if (ret >= 0) { + assert_se(size == strlen(output)); + assert_se(memcmp(buffer, output, size) == 0); + assert_se(((char*) buffer)[size] == 0); + } +} - assert_se(unbase64mem("Zm9vYmFy", strlen("Zm9vYmFy"), &mem, &len) == 0); - assert_se(streq(strndupa(mem, len), "foobar")); - free(mem); +static void test_unbase64mem(void) { - assert_se(unbase64mem("A", strlen("A"), &mem, &len) == -EINVAL); - assert_se(unbase64mem("A====", strlen("A===="), &mem, &len) == -EINVAL); - assert_se(unbase64mem("AAB==", strlen("AAB=="), &mem, &len) == -EINVAL); - assert_se(unbase64mem("AAAB=", strlen("AAAB="), &mem, &len) == -EINVAL); + test_unbase64mem_one("", "", 0); + test_unbase64mem_one("Zg==", "f", 0); + test_unbase64mem_one("Zm8=", "fo", 0); + test_unbase64mem_one("Zm9v", "foo", 0); + test_unbase64mem_one("Zm9vYg==", "foob", 0); + test_unbase64mem_one("Zm9vYmE=", "fooba", 0); + test_unbase64mem_one("Zm9vYmFy", "foobar", 0); + + test_unbase64mem_one(" ", "", 0); + test_unbase64mem_one(" \n\r ", "", 0); + test_unbase64mem_one(" Zg\n== ", "f", 0); + test_unbase64mem_one(" Zm 8=\r", "fo", 0); + test_unbase64mem_one(" Zm9\n\r\r\nv ", "foo", 0); + test_unbase64mem_one(" Z m9vYg==\n\r", "foob", 0); + test_unbase64mem_one(" Zm 9vYmE= ", "fooba", 0); + test_unbase64mem_one(" Z m9v YmFy ", "foobar", 0); + + test_unbase64mem_one("A", NULL, -EPIPE); + test_unbase64mem_one("A====", NULL, -EINVAL); + test_unbase64mem_one("AAB==", NULL, -EINVAL); + test_unbase64mem_one(" A A A B = ", NULL, -EINVAL); + test_unbase64mem_one(" Z m 8 = q u u x ", NULL, -ENAMETOOLONG); } static void test_hexdump(void) {