From 84319aa76e744c501d62052cc233e44dc48ece8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 26 Feb 2019 12:17:12 +0100 Subject: [PATCH 1/6] basic/utf8: change type of function to emphasize that it only looks at one character --- src/basic/utf8.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/basic/utf8.c b/src/basic/utf8.c index e0d1949dc7..ffe0adb33c 100644 --- a/src/basic/utf8.c +++ b/src/basic/utf8.c @@ -61,12 +61,7 @@ static bool unichar_is_control(char32_t ch) { } /* count of characters used to encode one unicode char */ -static size_t utf8_encoded_expected_len(const char *str) { - uint8_t c; - - assert(str); - - c = (uint8_t) str[0]; +static size_t utf8_encoded_expected_len(uint8_t c) { if (c < 0x80) return 1; if ((c & 0xe0) == 0xc0) @@ -90,7 +85,7 @@ int utf8_encoded_to_unichar(const char *str, char32_t *ret_unichar) { assert(str); - len = utf8_encoded_expected_len(str); + len = utf8_encoded_expected_len(str[0]); switch (len) { case 1: @@ -405,7 +400,7 @@ char16_t *utf8_to_utf16(const char *s, size_t length) { char32_t unichar; size_t e; - e = utf8_encoded_expected_len(s + i); + e = utf8_encoded_expected_len(s[i]); if (e <= 1) /* Invalid and single byte characters are copied as they are */ goto copy; @@ -464,7 +459,7 @@ int utf8_encoded_valid_unichar(const char *str) { assert(str); - len = utf8_encoded_expected_len(str); + len = utf8_encoded_expected_len(str[0]); if (len == 0) return -EINVAL; From 92e068b465299d5d88fe480bc5cf2bf4bd750516 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 26 Feb 2019 12:37:40 +0100 Subject: [PATCH 2/6] basic/utf8: do not read past end of string when looking for a multi-byte character Fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=9341. --- src/basic/device-nodes.c | 2 +- src/basic/utf8.c | 25 +++++++++++++------- src/basic/utf8.h | 2 +- src/libudev/libudev-util.c | 2 +- src/shared/ask-password-api.c | 12 +++++----- src/shared/json.c | 5 ++-- src/test/test-utf8.c | 20 ++++++++++++---- test/fuzz/fuzz-journal-remote/oss-fuzz-9341 | Bin 0 -> 1153 bytes 8 files changed, 42 insertions(+), 26 deletions(-) create mode 100644 test/fuzz/fuzz-journal-remote/oss-fuzz-9341 diff --git a/src/basic/device-nodes.c b/src/basic/device-nodes.c index 5fcdf24bd2..5ebe5b2483 100644 --- a/src/basic/device-nodes.c +++ b/src/basic/device-nodes.c @@ -28,7 +28,7 @@ int encode_devnode_name(const char *str, char *str_enc, size_t len) { for (i = 0, j = 0; str[i] != '\0'; i++) { int seqlen; - seqlen = utf8_encoded_valid_unichar(&str[i]); + seqlen = utf8_encoded_valid_unichar(str + i, (size_t) -1); if (seqlen > 1) { if (len-j < (size_t)seqlen) diff --git a/src/basic/utf8.c b/src/basic/utf8.c index ffe0adb33c..090c69d140 100644 --- a/src/basic/utf8.c +++ b/src/basic/utf8.c @@ -128,14 +128,14 @@ bool utf8_is_printable_newline(const char* str, size_t length, bool newline) { assert(str); - for (p = str; length;) { + for (p = str; length > 0;) { int encoded_len, r; char32_t val; - encoded_len = utf8_encoded_valid_unichar(p); - if (encoded_len < 0 || - (size_t) encoded_len > length) + encoded_len = utf8_encoded_valid_unichar(p, length); + if (encoded_len < 0) return false; + assert(encoded_len > 0 && (size_t) encoded_len <= length); r = utf8_encoded_to_unichar(p, &val); if (r < 0 || @@ -159,7 +159,7 @@ char *utf8_is_valid(const char *str) { while (*p) { int len; - len = utf8_encoded_valid_unichar(p); + len = utf8_encoded_valid_unichar(p, (size_t) -1); if (len < 0) return NULL; @@ -181,7 +181,7 @@ char *utf8_escape_invalid(const char *str) { while (*str) { int len; - len = utf8_encoded_valid_unichar(str); + len = utf8_encoded_valid_unichar(str, (size_t) -1); if (len > 0) { s = mempcpy(s, str, len); str += len; @@ -208,7 +208,7 @@ char *utf8_escape_non_printable(const char *str) { while (*str) { int len; - len = utf8_encoded_valid_unichar(str); + len = utf8_encoded_valid_unichar(str, (size_t) -1); if (len > 0) { if (utf8_is_printable(str, len)) { s = mempcpy(s, str, len); @@ -452,17 +452,24 @@ static int utf8_unichar_to_encoded_len(char32_t unichar) { } /* validate one encoded unicode char and return its length */ -int utf8_encoded_valid_unichar(const char *str) { +int utf8_encoded_valid_unichar(const char *str, size_t length /* bytes */) { char32_t unichar; size_t len, i; int r; assert(str); + assert(length > 0); + + /* We read until NUL, at most length bytes. (size_t) -1 may be used to disable the length check. */ len = utf8_encoded_expected_len(str[0]); if (len == 0) return -EINVAL; + /* Do we have a truncated multi-byte character? */ + if (len > length) + return -EINVAL; + /* ascii is valid */ if (len == 1) return 1; @@ -495,7 +502,7 @@ size_t utf8_n_codepoints(const char *str) { while (*str != 0) { int k; - k = utf8_encoded_valid_unichar(str); + k = utf8_encoded_valid_unichar(str, (size_t) -1); if (k < 0) return (size_t) -1; diff --git a/src/basic/utf8.h b/src/basic/utf8.h index 628456936e..6df70921db 100644 --- a/src/basic/utf8.h +++ b/src/basic/utf8.h @@ -32,7 +32,7 @@ char16_t *utf8_to_utf16(const char *s, size_t length); size_t char16_strlen(const char16_t *s); /* returns the number of 16bit words in the string (not bytes!) */ -int utf8_encoded_valid_unichar(const char *str); +int utf8_encoded_valid_unichar(const char *str, size_t length); int utf8_encoded_to_unichar(const char *str, char32_t *ret_unichar); static inline bool utf16_is_surrogate(char16_t c) { diff --git a/src/libudev/libudev-util.c b/src/libudev/libudev-util.c index 7e21719fbf..37660d0313 100644 --- a/src/libudev/libudev-util.c +++ b/src/libudev/libudev-util.c @@ -175,7 +175,7 @@ size_t util_replace_chars(char *str, const char *white) { } /* accept valid utf8 */ - len = utf8_encoded_valid_unichar(&str[i]); + len = utf8_encoded_valid_unichar(str + i, (size_t) -1); if (len > 1) { i += len; continue; diff --git a/src/shared/ask-password-api.c b/src/shared/ask-password-api.c index 072bf72c56..bc5e1cf669 100644 --- a/src/shared/ask-password-api.c +++ b/src/shared/ask-password-api.c @@ -385,13 +385,13 @@ int ask_password_tty( if (!(flags & ASK_PASSWORD_SILENT)) backspace_chars(ttyfd, 1); - /* Remove a full UTF-8 codepoint from the end. For that, figure out where the last one - * begins */ + /* Remove a full UTF-8 codepoint from the end. For that, figure out where the + * last one begins */ q = 0; for (;;) { size_t z; - z = utf8_encoded_valid_unichar(passphrase + q); + z = utf8_encoded_valid_unichar(passphrase + q, (size_t) -1); if (z == 0) { q = (size_t) -1; /* Invalid UTF8! */ break; @@ -410,8 +410,8 @@ int ask_password_tty( flags |= ASK_PASSWORD_SILENT; - /* There are two ways to enter silent mode. Either by pressing backspace as first key - * (and only as first key), or ... */ + /* There are two ways to enter silent mode. Either by pressing backspace as + * first key (and only as first key), or ... */ if (ttyfd >= 0) (void) loop_write(ttyfd, "(no echo) ", 10, false); @@ -440,7 +440,7 @@ int ask_password_tty( if (!(flags & ASK_PASSWORD_SILENT) && ttyfd >= 0) { /* Check if we got a complete UTF-8 character now. If so, let's output one '*'. */ - n = utf8_encoded_valid_unichar(passphrase + codepoint); + n = utf8_encoded_valid_unichar(passphrase + codepoint, (size_t) -1); if (n >= 0) { codepoint = p; (void) loop_write(ttyfd, (flags & ASK_PASSWORD_ECHO) ? &c : "*", 1, false); diff --git a/src/shared/json.c b/src/shared/json.c index 3786ff12b8..7ae1ffb1b2 100644 --- a/src/shared/json.c +++ b/src/shared/json.c @@ -1756,7 +1756,6 @@ static void inc_lines_columns(unsigned *line, unsigned *column, const char *s, s assert(s || n == 0); while (n > 0) { - if (*s == '\n') { (*line)++; *column = 1; @@ -1765,7 +1764,7 @@ static void inc_lines_columns(unsigned *line, unsigned *column, const char *s, s else { int w; - w = utf8_encoded_valid_unichar(s); + w = utf8_encoded_valid_unichar(s, n); if (w < 0) /* count invalid unichars as normal characters */ w = 1; else if ((size_t) w > n) /* never read more than the specified number of characters */ @@ -1930,7 +1929,7 @@ static int json_parse_string(const char **p, char **ret) { continue; } - len = utf8_encoded_valid_unichar(c); + len = utf8_encoded_valid_unichar(c, (size_t) -1); if (len < 0) return len; diff --git a/src/test/test-utf8.c b/src/test/test-utf8.c index 9849530ac8..d1e48da2a6 100644 --- a/src/test/test-utf8.c +++ b/src/test/test-utf8.c @@ -36,11 +36,21 @@ static void test_ascii_is_valid_n(void) { } static void test_utf8_encoded_valid_unichar(void) { - assert_se(utf8_encoded_valid_unichar("\342\204\242") == 3); - assert_se(utf8_encoded_valid_unichar("\302\256") == 2); - assert_se(utf8_encoded_valid_unichar("a") == 1); - assert_se(utf8_encoded_valid_unichar("\341\204") < 0); - assert_se(utf8_encoded_valid_unichar("\341\204\341\204") < 0); + assert_se(utf8_encoded_valid_unichar("\342\204\242", 1) == -EINVAL); /* truncated */ + assert_se(utf8_encoded_valid_unichar("\342\204\242", 2) == -EINVAL); /* truncated */ + assert_se(utf8_encoded_valid_unichar("\342\204\242", 3) == 3); + assert_se(utf8_encoded_valid_unichar("\342\204\242", 4) == 3); + assert_se(utf8_encoded_valid_unichar("\302\256", 1) == -EINVAL); /* truncated */ + assert_se(utf8_encoded_valid_unichar("\302\256", 2) == 2); + assert_se(utf8_encoded_valid_unichar("\302\256", 3) == 2); + assert_se(utf8_encoded_valid_unichar("\302\256", (size_t) -1) == 2); + assert_se(utf8_encoded_valid_unichar("a", 1) == 1); + assert_se(utf8_encoded_valid_unichar("a", 2) == 1); + assert_se(utf8_encoded_valid_unichar("\341\204", 1) == -EINVAL); /* truncated, potentially valid */ + assert_se(utf8_encoded_valid_unichar("\341\204", 2) == -EINVAL); /* truncated, potentially valid */ + assert_se(utf8_encoded_valid_unichar("\341\204", 3) == -EINVAL); + assert_se(utf8_encoded_valid_unichar("\341\204\341\204", 4) == -EINVAL); + assert_se(utf8_encoded_valid_unichar("\341\204\341\204", 5) == -EINVAL); } static void test_utf8_escaping(void) { diff --git a/test/fuzz/fuzz-journal-remote/oss-fuzz-9341 b/test/fuzz/fuzz-journal-remote/oss-fuzz-9341 new file mode 100644 index 0000000000000000000000000000000000000000..3ddac6b8951a29bef113c261980401d7c8afe03b GIT binary patch literal 1153 zcmcCj00XZ0_#jtDpAb)9*LV;S9OCF3U<;BmF|af>;ex0Fkw8H%pnR~SyDL~8A%o1~ x90j8xFd70QF$7EuObiVSfyl(d*wpX~my2VFqpblPaB;N literal 0 HcmV?d00001 From d26eef9252677bfe06ed76eedc11db3001283814 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 26 Feb 2019 12:43:07 +0100 Subject: [PATCH 3/6] shared/ask-password-api: when echoing multi-byte characters, print the whole sequence This is untested, but I don't see how the previous code could have worked for multibyte characters (with echo on). --- src/shared/ask-password-api.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/shared/ask-password-api.c b/src/shared/ask-password-api.c index bc5e1cf669..a3d61d5b9a 100644 --- a/src/shared/ask-password-api.c +++ b/src/shared/ask-password-api.c @@ -442,8 +442,11 @@ int ask_password_tty( /* Check if we got a complete UTF-8 character now. If so, let's output one '*'. */ n = utf8_encoded_valid_unichar(passphrase + codepoint, (size_t) -1); if (n >= 0) { + if (flags & ASK_PASSWORD_ECHO) + (void) loop_write(ttyfd, passphrase + codepoint, n, false); + else + (void) loop_write(ttyfd, "*", 1, false); codepoint = p; - (void) loop_write(ttyfd, (flags & ASK_PASSWORD_ECHO) ? &c : "*", 1, false); } } From eafadd069c4e30ed62173123326a7237448615d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 26 Feb 2019 13:00:35 +0100 Subject: [PATCH 4/6] fuzz-journal-stream: avoid assertion failure on samples which don't fit in pipe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=11587. We had a sample which was large enough that write(2) failed to push all the data into the pipe, and an assert failed. The code could be changed to use a loop, but then we'd need to interleave writes and sd_event_run (to process the journal). I don't think the complexity is worth it — fuzzing works best if the sample is not too huge anyway. So let's just reject samples above 64k, and tell oss-fuzz about this limit. --- src/fuzz/fuzz-journald-stream.c | 2 +- src/fuzz/fuzz-journald-stream.options | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) create mode 100644 src/fuzz/fuzz-journald-stream.options diff --git a/src/fuzz/fuzz-journald-stream.c b/src/fuzz/fuzz-journald-stream.c index 5d6c8eb8ca..bc53964139 100644 --- a/src/fuzz/fuzz-journald-stream.c +++ b/src/fuzz/fuzz-journald-stream.c @@ -15,7 +15,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { StdoutStream *stream; int v; - if (size == 0) + if (size == 0 || size > 65536) return 0; if (!getenv("SYSTEMD_LOG_LEVEL")) diff --git a/src/fuzz/fuzz-journald-stream.options b/src/fuzz/fuzz-journald-stream.options new file mode 100644 index 0000000000..678d526b1e --- /dev/null +++ b/src/fuzz/fuzz-journald-stream.options @@ -0,0 +1,2 @@ +[libfuzzer] +max_len = 65536 From d92e7b8fe46e450726106c7289d5ce36f6c3c611 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 26 Feb 2019 13:12:53 +0100 Subject: [PATCH 5/6] fuzz-lldp: avoid assertion failure on samples which dont fit in pipe Fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=11603. --- src/fuzz/fuzz-lldp.c | 3 +++ src/fuzz/fuzz-lldp.options | 2 ++ 2 files changed, 5 insertions(+) create mode 100644 src/fuzz/fuzz-lldp.options diff --git a/src/fuzz/fuzz-lldp.c b/src/fuzz/fuzz-lldp.c index b9291d4783..7f252309cd 100644 --- a/src/fuzz/fuzz-lldp.c +++ b/src/fuzz/fuzz-lldp.c @@ -23,6 +23,9 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { _cleanup_(sd_event_unrefp) sd_event *e = NULL; _cleanup_(sd_lldp_unrefp) sd_lldp *lldp = NULL; + if (size > 2048) + return 0; + assert_se(sd_event_new(&e) == 0); assert_se(sd_lldp_new(&lldp) >= 0); assert_se(sd_lldp_set_ifindex(lldp, 42) >= 0); diff --git a/src/fuzz/fuzz-lldp.options b/src/fuzz/fuzz-lldp.options new file mode 100644 index 0000000000..60bd9b0b2f --- /dev/null +++ b/src/fuzz/fuzz-lldp.options @@ -0,0 +1,2 @@ +[libfuzzer] +max_len = 2048 From 9b7f73b03f99d9375506c7d54e8a993d027e4506 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 26 Feb 2019 13:12:53 +0100 Subject: [PATCH 6/6] fuzz-ndisc-rs: avoid assertion failure on samples which dont fit in pipe Fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=11605. --- src/fuzz/fuzz-ndisc-rs.c | 3 +++ src/fuzz/fuzz-ndisc-rs.options | 2 ++ 2 files changed, 5 insertions(+) create mode 100644 src/fuzz/fuzz-ndisc-rs.options diff --git a/src/fuzz/fuzz-ndisc-rs.c b/src/fuzz/fuzz-ndisc-rs.c index 3a1e60fc5d..d8d256d127 100644 --- a/src/fuzz/fuzz-ndisc-rs.c +++ b/src/fuzz/fuzz-ndisc-rs.c @@ -43,6 +43,9 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { _cleanup_(sd_event_unrefp) sd_event *e = NULL; _cleanup_(sd_ndisc_unrefp) sd_ndisc *nd = NULL; + if (size > 2048) + return 0; + assert_se(sd_event_new(&e) >= 0); assert_se(sd_ndisc_new(&nd) >= 0); assert_se(sd_ndisc_attach_event(nd, e, 0) >= 0); diff --git a/src/fuzz/fuzz-ndisc-rs.options b/src/fuzz/fuzz-ndisc-rs.options new file mode 100644 index 0000000000..60bd9b0b2f --- /dev/null +++ b/src/fuzz/fuzz-ndisc-rs.options @@ -0,0 +1,2 @@ +[libfuzzer] +max_len = 2048