Merge pull request #9827 from yuwata/fix-9795-9820

journal: fixes issues reported by ASan

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 (k == 3) {
-        } else if ((*p)[3] == '>') {
                 b = undecchar((*p)[1]);
                 c = undecchar((*p)[2]);
-                k = 4;
+        } else if (k == 4) {
-        } else if ((*p)[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;
 }
 

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)))

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 {

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 += strspn(p + e, WHITESPACE);
-                e++;
+
         *buf = p + e;
         return e;
 }
@@ -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;
@@ -333,13 +334,21 @@ 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 {
-                msg = t = newa(char, i - leading_ws + 1);
+                msg = dummy = new(char, i - leading_ws + 1);
-                memcpy(t, buf + leading_ws, i - leading_ws);
+                if (!dummy) {
-                t[i - leading_ws] = 0;
+                        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
@@ -397,24 +406,33 @@ 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);
 
                 iovec[n++] = IOVEC_MAKE(t, hlen + syslog_ts_len);
         }
 
-        a = strjoina("MESSAGE=", msg);
+        msg_msg = strjoin("MESSAGE=", msg);
-        iovec[n++] = IOVEC_MAKE_STRING(a);
+        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);
+                msg_raw = new(char, hlen + raw_len);
-                memcpy(t, "SYSLOG_RAW=", hlen);
+                if (!msg_raw) {
-                memcpy(t + hlen, buf, raw_len);
+                        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);

src/journal/test-journal-syslog.c

@@ -4,9 +4,10 @@
 #include "journald-syslog.h"
 #include "macro.h"
 #include "string-util.h"
+#include "syslog-util.h"
 
-static void test_syslog_parse_identifier(const char* str,
+static void test_syslog_parse_identifier(const char *str,
-                                         const char *ident, const char*pid, int ret) {
+                                         const char *ident, const char *pid, int ret) {
         const char *buf = str;
         _cleanup_free_ char *ident2 = NULL, *pid2 = NULL;
         int ret2;
@@ -18,10 +19,35 @@ 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);
+        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);
+
+        test_syslog_parse_priority("<>", 0, 0);
+        test_syslog_parse_priority("<>aaa", 0, 0);
+        test_syslog_parse_priority("<aaaa>", 0, 0);
+        test_syslog_parse_priority("<aaaa>aaa", 0, 0);
+        test_syslog_parse_priority(" <aaaa>", 0, 0);
+        test_syslog_parse_priority(" <aaaa>aaa", 0, 0);
+        /* TODO: add test cases of valid priorities */
 
         return 0;
 }

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);

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;
 }