From eba87a17dad09ff0e77eadb7fa25db1214a095fd Mon Sep 17 00:00:00 2001 From: Filipe Brandenburger Date: Thu, 10 Jan 2019 14:53:33 -0800 Subject: [PATCH 1/2] journal: fix out-of-bounds read CVE-2018-16866 The original code didn't account for the fact that strchr() would match on the '\0' character, making it read past the end of the buffer if no non-whitespace character was present. This bug was introduced in commit ec5ff4445cca6a which was first released in systemd v221 and later fixed in commit 8595102d3ddde6 which was released in v240, so versions in the range [v221, v240) are affected. --- 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 8e034c8fa9c..2d2c1b9db2c 100644 --- a/src/journal/journald-syslog.c +++ b/src/journal/journald-syslog.c @@ -238,7 +238,7 @@ size_t syslog_parse_identifier(const char **buf, char **identifier, char **pid) if (t) *identifier = t; - if (strchr(WHITESPACE, p[e])) + if (p[e] != '\0' && strchr(WHITESPACE, p[e])) e++; *buf = p + e; return e; From afc78ce388a730cd094c63ade1cebf620cd6fea0 Mon Sep 17 00:00:00 2001 From: Filipe Brandenburger Date: Thu, 10 Jan 2019 15:35:57 -0800 Subject: [PATCH 2/2] journal: Add test cases that catch out-of-bounds read in journald The test cases from commit 8595102d3ddde6 check for the return value of syslog_parse_identifier() and will catch the condition that produced vulnerability from CVE-2018-16866. Add these tests to our stable branches. Tested that these tests will fail if the fix for CVE-2018-16866 is missing from the branch. --- src/journal/test-journal-syslog.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/journal/test-journal-syslog.c b/src/journal/test-journal-syslog.c index 4ff7f3ec2ef..3378f09e475 100644 --- a/src/journal/test-journal-syslog.c +++ b/src/journal/test-journal-syslog.c @@ -22,8 +22,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, const char *rest, int ret) { const char *buf = str; _cleanup_free_ char *ident2 = NULL, *pid2 = NULL; int ret2; @@ -33,12 +33,22 @@ static void test_syslog_parse_identifier(const char* str, assert_se(ret == ret2); assert_se(ident == ident2 || streq_ptr(ident, ident2)); assert_se(pid == pid2 || streq_ptr(pid, pid2)); + assert_se(streq(buf, rest)); } 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", NULL, NULL, 0); + test_syslog_parse_identifier("pidu[111]: xxx", "pidu", "111", "xxx", 11); + test_syslog_parse_identifier("pidu: xxx", "pidu", NULL, "xxx", 6); + test_syslog_parse_identifier("pidu: xxx", "pidu", NULL, " xxx", 6); + test_syslog_parse_identifier("pidu xxx", NULL, NULL, "pidu xxx", 0); + test_syslog_parse_identifier(" pidu xxx", NULL, NULL, " pidu xxx", 0); + test_syslog_parse_identifier("", NULL, NULL, "", 0); + test_syslog_parse_identifier(" ", NULL, NULL, " ", 0); + test_syslog_parse_identifier(":", "", NULL, "", 1); + test_syslog_parse_identifier(": ", "", NULL, " ", 2); + test_syslog_parse_identifier("pidu:", "pidu", NULL, "", 5); + test_syslog_parse_identifier("pidu: ", "pidu", NULL, "", 6); + test_syslog_parse_identifier("pidu : ", NULL, NULL, "pidu : ", 0); return 0; }