diff options
| -rw-r--r-- | src/basic/string-util.c | 28 | ||||
| -rw-r--r-- | src/basic/string-util.h | 1 | ||||
| -rw-r--r-- | src/libsystemd/sd-bus/bus-message.c | 34 | ||||
| -rw-r--r-- | src/test/test-string-util.c | 62 | ||||
| -rw-r--r-- | test/fuzz/fuzz-bus-message/crash-b88ad9ecf4aacf4a0caca5b5543953265367f084 | bin | 0 -> 32 bytes |
5 files changed, 103 insertions, 22 deletions
diff --git a/src/basic/string-util.c b/src/basic/string-util.c index 0a40683493..dfa739996f 100644 --- a/src/basic/string-util.c +++ b/src/basic/string-util.c @@ -1004,7 +1004,7 @@ int free_and_strdup(char **p, const char *s) { assert(p); - /* Replaces a string pointer with an strdup()ed new string, + /* Replaces a string pointer with a strdup()ed new string, * possibly freeing the old one. */ if (streq_ptr(*p, s)) @@ -1023,6 +1023,32 @@ int free_and_strdup(char **p, const char *s) { return 1; } +int free_and_strndup(char **p, const char *s, size_t l) { + char *t; + + assert(p); + assert(s || l == 0); + + /* Replaces a string pointer with a strndup()ed new string, + * freeing the old one. */ + + if (!*p && !s) + return 0; + + if (*p && s && strneq(*p, s, l) && (l > strlen(*p) || (*p)[l] == '\0')) + return 0; + + if (s) { + t = strndup(s, l); + if (!t) + return -ENOMEM; + } else + t = NULL; + + free_and_replace(*p, t); + return 1; +} + #if !HAVE_EXPLICIT_BZERO /* * Pointer to memset is volatile so that compiler must de-reference diff --git a/src/basic/string-util.h b/src/basic/string-util.h index fcd12f3a30..72c075aa39 100644 --- a/src/basic/string-util.h +++ b/src/basic/string-util.h @@ -176,6 +176,7 @@ char *strrep(const char *s, unsigned n); int split_pair(const char *s, const char *sep, char **l, char **r); int free_and_strdup(char **p, const char *s); +int free_and_strndup(char **p, const char *s, size_t l); /* Normal memmem() requires haystack to be nonnull, which is annoying for zero-length buffers */ static inline void *memmem_safe(const void *haystack, size_t haystacklen, const void *needle, size_t needlelen) { diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c index 6e404367db..91bcf49a48 100644 --- a/src/libsystemd/sd-bus/bus-message.c +++ b/src/libsystemd/sd-bus/bus-message.c @@ -4144,20 +4144,19 @@ _public_ int sd_bus_message_peek_type(sd_bus_message *m, char *type, const char if (contents) { size_t l; - char *sig; r = signature_element_length(c->signature+c->index+1, &l); if (r < 0) return r; - assert(l >= 1); + /* signature_element_length does verification internally */ - sig = strndup(c->signature + c->index + 1, l); - if (!sig) + assert(l >= 1); + if (free_and_strndup(&c->peeked_signature, + c->signature + c->index + 1, l) < 0) return -ENOMEM; - free(c->peeked_signature); - *contents = c->peeked_signature = sig; + *contents = c->peeked_signature; } if (type) @@ -4170,19 +4169,17 @@ _public_ int sd_bus_message_peek_type(sd_bus_message *m, char *type, const char if (contents) { size_t l; - char *sig; r = signature_element_length(c->signature+c->index, &l); if (r < 0) return r; assert(l >= 2); - sig = strndup(c->signature + c->index + 1, l - 2); - if (!sig) + if (free_and_strndup(&c->peeked_signature, + c->signature + c->index + 1, l - 2) < 0) return -ENOMEM; - free(c->peeked_signature); - *contents = c->peeked_signature = sig; + *contents = c->peeked_signature; } if (type) @@ -4222,9 +4219,8 @@ _public_ int sd_bus_message_peek_type(sd_bus_message *m, char *type, const char if (k > c->item_size) return -EBADMSG; - free(c->peeked_signature); - c->peeked_signature = strndup((char*) q + 1, k - 1); - if (!c->peeked_signature) + if (free_and_strndup(&c->peeked_signature, + (char*) q + 1, k - 1) < 0) return -ENOMEM; if (!signature_is_valid(c->peeked_signature, true)) @@ -5056,25 +5052,21 @@ int bus_message_parse_fields(sd_bus_message *m) { if (*p == 0) { size_t l; - char *c; /* We found the beginning of the signature * string, yay! We require the body to be a * structure, so verify it and then strip the * opening/closing brackets. */ - l = ((char*) m->footer + m->footer_accessible) - p - (1 + sz); + l = (char*) m->footer + m->footer_accessible - p - (1 + sz); if (l < 2 || p[1] != SD_BUS_TYPE_STRUCT_BEGIN || p[1 + l - 1] != SD_BUS_TYPE_STRUCT_END) return -EBADMSG; - c = strndup(p + 1 + 1, l - 2); - if (!c) + if (free_and_strndup(&m->root_container.signature, + p + 1 + 1, l - 2) < 0) return -ENOMEM; - - free(m->root_container.signature); - m->root_container.signature = c; break; } diff --git a/src/test/test-string-util.c b/src/test/test-string-util.c index a9f261839c..8c1f91d4ef 100644 --- a/src/test/test-string-util.c +++ b/src/test/test-string-util.c @@ -5,6 +5,7 @@ #include "macro.h" #include "string-util.h" #include "strv.h" +#include "tests.h" #include "utf8.h" static void test_string_erase(void) { @@ -30,6 +31,64 @@ static void test_string_erase(void) { assert_se(x[9] == '\0'); } +static void test_free_and_strndup_one(char **t, const char *src, size_t l, const char *expected, bool change) { + int r; + + log_debug("%s: \"%s\", \"%s\", %zd (expect \"%s\", %s)", + __func__, strnull(*t), strnull(src), l, strnull(expected), yes_no(change)); + + r = free_and_strndup(t, src, l); + assert_se(streq_ptr(*t, expected)); + assert_se(r == change); /* check that change occurs only when necessary */ +} + +static void test_free_and_strndup(void) { + static const struct test_case { + const char *src; + size_t len; + const char *expected; + } cases[] = { + {"abc", 0, ""}, + {"abc", 0, ""}, + {"abc", 1, "a"}, + {"abc", 2, "ab"}, + {"abc", 3, "abc"}, + {"abc", 4, "abc"}, + {"abc", 5, "abc"}, + {"abc", 5, "abc"}, + {"abc", 4, "abc"}, + {"abc", 3, "abc"}, + {"abc", 2, "ab"}, + {"abc", 1, "a"}, + {"abc", 0, ""}, + + {"", 0, ""}, + {"", 1, ""}, + {"", 2, ""}, + {"", 0, ""}, + {"", 1, ""}, + {"", 2, ""}, + {"", 2, ""}, + {"", 1, ""}, + {"", 0, ""}, + + {NULL, 0, NULL}, + + {"foo", 3, "foo"}, + {"foobar", 6, "foobar"}, + }; + + _cleanup_free_ char *t = NULL; + const char *prev_expected = t; + + for (unsigned i = 0; i < ELEMENTSOF(cases); i++) { + test_free_and_strndup_one(&t, + cases[i].src, cases[i].len, cases[i].expected, + !streq_ptr(cases[i].expected, prev_expected)); + prev_expected = t; + } +} + static void test_ascii_strcasecmp_n(void) { assert_se(ascii_strcasecmp_n("", "", 0) == 0); @@ -520,7 +579,10 @@ static void test_memory_startswith_no_case(void) { } int main(int argc, char *argv[]) { + test_setup_logging(LOG_DEBUG); + test_string_erase(); + test_free_and_strndup(); test_ascii_strcasecmp_n(); test_ascii_strcasecmp_nn(); test_cellescape(); diff --git a/test/fuzz/fuzz-bus-message/crash-b88ad9ecf4aacf4a0caca5b5543953265367f084 b/test/fuzz/fuzz-bus-message/crash-b88ad9ecf4aacf4a0caca5b5543953265367f084 Binary files differnew file mode 100644 index 0000000000..52469650b5 --- /dev/null +++ b/test/fuzz/fuzz-bus-message/crash-b88ad9ecf4aacf4a0caca5b5543953265367f084 |