From 57019d5f75ca4ce6e82161447ff3ed0745d8a77a Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 8 Aug 2018 12:40:44 +0900 Subject: [PATCH 1/8] journal: do not pass a negative value to memcpy() The message may contains only whitespaces. Fixes #9795. --- src/journal/journald-syslog.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/journal/journald-syslog.c b/src/journal/journald-syslog.c index 08b4fcb1c2..2de3bdf66f 100644 --- a/src/journal/journald-syslog.c +++ b/src/journal/journald-syslog.c @@ -333,7 +333,10 @@ void server_process_syslog_message( leading_ws = strspn(buf, WHITESPACE); - if (i == raw_len) + if (i == 0) + /* The message contains only whitespaces */ + msg = buf + raw_len; + else if (i == raw_len) /* Nice! No need to strip anything on the end, let's optimize this a bit */ msg = buf + leading_ws; else { From 75f8d86eea45bfc862f65f4fca61457fc086153d Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 8 Aug 2018 12:50:39 +0900 Subject: [PATCH 2/8] journal: do not use newa() or strjoina() for message Fixes another issue reported in #9795. --- src/journal/journald-syslog.c | 35 +++++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/src/journal/journald-syslog.c b/src/journal/journald-syslog.c index 2de3bdf66f..25f2148023 100644 --- a/src/journal/journald-syslog.c +++ b/src/journal/journald-syslog.c @@ -304,7 +304,8 @@ void server_process_syslog_message( char *t, syslog_priority[sizeof("PRIORITY=") + DECIMAL_STR_MAX(int)], syslog_facility[sizeof("SYSLOG_FACILITY=") + DECIMAL_STR_MAX(int)]; const char *msg, *syslog_ts, *a; - _cleanup_free_ char *identifier = NULL, *pid = NULL; + _cleanup_free_ char *identifier = NULL, *pid = NULL, + *dummy = NULL, *msg_msg = NULL, *msg_raw = NULL; int priority = LOG_USER | LOG_INFO, r; ClientContext *context = NULL; struct iovec *iovec; @@ -340,9 +341,14 @@ void server_process_syslog_message( /* Nice! No need to strip anything on the end, let's optimize this a bit */ msg = buf + leading_ws; else { - msg = t = newa(char, i - leading_ws + 1); - memcpy(t, buf + leading_ws, i - leading_ws); - t[i - leading_ws] = 0; + msg = dummy = new(char, i - leading_ws + 1); + if (!dummy) { + log_oom(); + return; + } + + memcpy(dummy, buf + leading_ws, i - leading_ws); + dummy[i - leading_ws] = 0; } /* We will add the SYSLOG_RAW= field when we stripped anything @@ -407,17 +413,26 @@ void server_process_syslog_message( iovec[n++] = IOVEC_MAKE(t, hlen + syslog_ts_len); } - a = strjoina("MESSAGE=", msg); - iovec[n++] = IOVEC_MAKE_STRING(a); + msg_msg = strjoin("MESSAGE=", msg); + if (!msg_msg) { + log_oom(); + return; + } + iovec[n++] = IOVEC_MAKE_STRING(msg_msg); if (store_raw) { const size_t hlen = strlen("SYSLOG_RAW="); - t = newa(char, hlen + raw_len); - memcpy(t, "SYSLOG_RAW=", hlen); - memcpy(t + hlen, buf, raw_len); + msg_raw = new(char, hlen + raw_len); + if (!msg_raw) { + log_oom(); + return; + } - iovec[n++] = IOVEC_MAKE(t, hlen + raw_len); + memcpy(msg_raw, "SYSLOG_RAW=", hlen); + memcpy(msg_raw + hlen, buf, raw_len); + + iovec[n++] = IOVEC_MAKE(msg_raw, hlen + raw_len); } server_dispatch_message(s, iovec, n, m, context, tv, priority, 0); From a6aadf4ae0bae185dc4c414d492a4a781c80ffe5 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 8 Aug 2018 15:06:36 +0900 Subject: [PATCH 3/8] journal: fix syslog_parse_identifier() Fixes #9829. --- src/journal/journald-syslog.c | 6 +++--- src/journal/test-journal-syslog.c | 10 ++++++++-- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/journal/journald-syslog.c b/src/journal/journald-syslog.c index 25f2148023..dc192d5a70 100644 --- a/src/journal/journald-syslog.c +++ b/src/journal/journald-syslog.c @@ -193,7 +193,7 @@ size_t syslog_parse_identifier(const char **buf, char **identifier, char **pid) e = l; l--; - if (p[l-1] == ']') { + if (l > 0 && p[l-1] == ']') { size_t k = l-1; for (;;) { @@ -218,8 +218,8 @@ size_t syslog_parse_identifier(const char **buf, char **identifier, char **pid) if (t) *identifier = t; - if (strchr(WHITESPACE, p[e])) - e++; + e += strspn(p + e, WHITESPACE); + *buf = p + e; return e; } diff --git a/src/journal/test-journal-syslog.c b/src/journal/test-journal-syslog.c index 9ba86f6c8a..05f759817e 100644 --- a/src/journal/test-journal-syslog.c +++ b/src/journal/test-journal-syslog.c @@ -5,8 +5,8 @@ #include "macro.h" #include "string-util.h" -static void test_syslog_parse_identifier(const char* str, - const char *ident, const char*pid, int ret) { +static void test_syslog_parse_identifier(const char *str, + const char *ident, const char *pid, int ret) { const char *buf = str; _cleanup_free_ char *ident2 = NULL, *pid2 = NULL; int ret2; @@ -21,7 +21,13 @@ static void test_syslog_parse_identifier(const char* str, int main(void) { test_syslog_parse_identifier("pidu[111]: xxx", "pidu", "111", 11); test_syslog_parse_identifier("pidu: xxx", "pidu", NULL, 6); + test_syslog_parse_identifier("pidu: xxx", "pidu", NULL, 7); test_syslog_parse_identifier("pidu xxx", NULL, NULL, 0); + test_syslog_parse_identifier(":", "", NULL, 1); + test_syslog_parse_identifier(": ", "", NULL, 3); + test_syslog_parse_identifier("pidu:", "pidu", NULL, 5); + test_syslog_parse_identifier("pidu: ", "pidu", NULL, 6); + test_syslog_parse_identifier("pidu : ", NULL, NULL, 0); return 0; } From 7e8d494b33cf593e332f3c710c9c26aee4b88dd9 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 8 Aug 2018 15:52:46 +0900 Subject: [PATCH 4/8] core: use memcpy_safe() Fixes #9738. --- src/core/execute.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/execute.c b/src/core/execute.c index a35dbac9ef..ecf0f7c404 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -3216,7 +3216,7 @@ static int exec_child( } fds_with_exec_fd = newa(int, n_fds + 1); - memcpy(fds_with_exec_fd, fds, n_fds * sizeof(int)); + memcpy_safe(fds_with_exec_fd, fds, n_fds * sizeof(int)); fds_with_exec_fd[n_fds] = exec_fd; n_fds_with_exec_fd = n_fds + 1; } else { From f30faf854b9bf01da294547a1bc3660506d750db Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 8 Aug 2018 16:22:55 +0900 Subject: [PATCH 5/8] util: introduce memcmp_safe() --- src/basic/util.h | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/basic/util.h b/src/basic/util.h index 42c262f598..308933ac90 100644 --- a/src/basic/util.h +++ b/src/basic/util.h @@ -112,9 +112,7 @@ static inline void qsort_r_safe(void *base, size_t nmemb, size_t size, int (*com qsort_r(base, nmemb, size, compar, userdata); } -/** - * Normal memcpy requires src to be nonnull. We do nothing if n is 0. - */ +/* Normal memcpy requires src to be nonnull. We do nothing if n is 0. */ static inline void memcpy_safe(void *dst, const void *src, size_t n) { if (n == 0) return; @@ -122,6 +120,15 @@ static inline void memcpy_safe(void *dst, const void *src, size_t n) { memcpy(dst, src, n); } +/* Normal memcmp requires s1 and s2 to be nonnull. We do nothing if n is 0. */ +static inline int memcmp_safe(const void *s1, const void *s2, size_t n) { + if (n == 0) + return 0; + assert(s1); + assert(s2); + return memcmp(s1, s2, n); +} + int on_ac_power(void); #define memzero(x,l) (memset((x), 0, (l))) From 1f66559ca214c0f8975e99c77379d962d3253502 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 8 Aug 2018 16:23:20 +0900 Subject: [PATCH 6/8] resolve: use memcmp_safe() and memcpy_safe() As the length of salt in NSEC3 may be zero. Fixes #9757. --- src/resolve/resolved-dns-dnssec.c | 2 +- src/resolve/resolved-dns-packet.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/resolve/resolved-dns-dnssec.c b/src/resolve/resolved-dns-dnssec.c index e0d70c29cb..2c8514a72a 100644 --- a/src/resolve/resolved-dns-dnssec.c +++ b/src/resolve/resolved-dns-dnssec.c @@ -1380,7 +1380,7 @@ static int nsec3_is_good(DnsResourceRecord *rr, DnsResourceRecord *nsec3) { return 0; if (rr->nsec3.salt_size != nsec3->nsec3.salt_size) return 0; - if (memcmp(rr->nsec3.salt, nsec3->nsec3.salt, rr->nsec3.salt_size) != 0) + if (memcmp_safe(rr->nsec3.salt, nsec3->nsec3.salt, rr->nsec3.salt_size) != 0) return 0; a = dns_resource_key_name(rr->key); diff --git a/src/resolve/resolved-dns-packet.c b/src/resolve/resolved-dns-packet.c index 2cc606f363..6a615e3762 100644 --- a/src/resolve/resolved-dns-packet.c +++ b/src/resolve/resolved-dns-packet.c @@ -383,7 +383,7 @@ int dns_packet_append_blob(DnsPacket *p, const void *d, size_t l, size_t *start) if (r < 0) return r; - memcpy(q, d, l); + memcpy_safe(q, d, l); return 0; } From b9b8874f8fd541a71491cb0cf44cf72afcbcf0db Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 8 Aug 2018 12:48:47 +0900 Subject: [PATCH 7/8] journal: fix size of buffer --- src/journal/journald-syslog.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/journal/journald-syslog.c b/src/journal/journald-syslog.c index dc192d5a70..031c7adee5 100644 --- a/src/journal/journald-syslog.c +++ b/src/journal/journald-syslog.c @@ -406,7 +406,7 @@ void server_process_syslog_message( if (syslog_ts_len > 0) { const size_t hlen = strlen("SYSLOG_TIMESTAMP="); - t = newa(char, hlen + raw_len); + t = newa(char, hlen + syslog_ts_len); memcpy(t, "SYSLOG_TIMESTAMP=", hlen); memcpy(t + hlen, syslog_ts, syslog_ts_len); From a5ee33b951cfa22db53d0274c9c6c0d9d4dae39d Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 8 Aug 2018 18:27:15 +0900 Subject: [PATCH 8/8] syslog: fix segfault in syslog_parse_priority() --- src/basic/syslog-util.c | 20 +++++++++++--------- src/journal/test-journal-syslog.c | 20 ++++++++++++++++++++ 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/src/basic/syslog-util.c b/src/basic/syslog-util.c index 21461fa581..fe129482f3 100644 --- a/src/basic/syslog-util.c +++ b/src/basic/syslog-util.c @@ -10,7 +10,8 @@ int syslog_parse_priority(const char **p, int *priority, bool with_facility) { int a = 0, b = 0, c = 0; - int k; + const char *end; + size_t k; assert(p); assert(*p); @@ -19,21 +20,22 @@ int syslog_parse_priority(const char **p, int *priority, bool with_facility) { if ((*p)[0] != '<') return 0; - if (!strchr(*p, '>')) + end = strchr(*p, '>'); + if (!end) return 0; - if ((*p)[2] == '>') { + k = end - *p; + assert(k > 0); + + if (k == 2) c = undecchar((*p)[1]); - k = 3; - } else if ((*p)[3] == '>') { + else if (k == 3) { b = undecchar((*p)[1]); c = undecchar((*p)[2]); - k = 4; - } else if ((*p)[4] == '>') { + } else if (k == 4) { a = undecchar((*p)[1]); b = undecchar((*p)[2]); c = undecchar((*p)[3]); - k = 5; } else return 0; @@ -46,7 +48,7 @@ int syslog_parse_priority(const char **p, int *priority, bool with_facility) { else *priority = (*priority & LOG_FACMASK) | c; - *p += k; + *p += k + 1; return 1; } diff --git a/src/journal/test-journal-syslog.c b/src/journal/test-journal-syslog.c index 05f759817e..07b6cea3cb 100644 --- a/src/journal/test-journal-syslog.c +++ b/src/journal/test-journal-syslog.c @@ -4,6 +4,7 @@ #include "journald-syslog.h" #include "macro.h" #include "string-util.h" +#include "syslog-util.h" static void test_syslog_parse_identifier(const char *str, const char *ident, const char *pid, int ret) { @@ -18,6 +19,17 @@ static void test_syslog_parse_identifier(const char *str, assert_se(pid == pid2 || streq_ptr(pid, pid2)); } +static void test_syslog_parse_priority(const char *str, int priority, int ret) { + const char *buf = str; + int priority2, ret2; + + ret2 = syslog_parse_priority(&buf, &priority2, false); + + assert_se(ret == ret2); + if (ret2 == 1) + assert_se(priority == priority2); +} + int main(void) { test_syslog_parse_identifier("pidu[111]: xxx", "pidu", "111", 11); test_syslog_parse_identifier("pidu: xxx", "pidu", NULL, 6); @@ -29,5 +41,13 @@ int main(void) { test_syslog_parse_identifier("pidu: ", "pidu", NULL, 6); test_syslog_parse_identifier("pidu : ", NULL, NULL, 0); + test_syslog_parse_priority("<>", 0, 0); + test_syslog_parse_priority("<>aaa", 0, 0); + test_syslog_parse_priority("", 0, 0); + test_syslog_parse_priority("aaa", 0, 0); + test_syslog_parse_priority(" ", 0, 0); + test_syslog_parse_priority(" aaa", 0, 0); + /* TODO: add test cases of valid priorities */ + return 0; }