From 4489ac6683386805742f7ee678cb8580d669556b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Wed, 5 Dec 2018 22:45:02 +0100
Subject: [PATCH 06/11] journald: set a limit on the number of fields (1k)
We allocate a iovec entry for each field, so with many short entries,
our memory usage and processing time can be large, even with a relatively
small message size. Let's refuse overly long entries.
CVE-2018-16865
https://bugzilla.redhat.com/show_bug.cgi?id=1653861
What from I can see, the problem is not from an alloca, despite what the CVE
description says, but from the attack multiplication that comes from creating
many very small iovecs: (void* + size_t) for each three bytes of input message.
---
src/journal/journald-native.c | 5 +++++
src/shared/journal-importer.h | 3 +++
2 files changed, 8 insertions(+)
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Fri, 7 Dec 2018 10:48:10 +0100
Subject: [PATCH 11/11] journal-remote: set a limit on the number of fields in
a message
Existing use of E2BIG is replaced with ENOBUFS (entry too long), and E2BIG is
reused for the new error condition (too many fields).
This matches the change done for systemd-journald, hence forming the second
part of the fix for CVE-2018-16865
(https://bugzilla.redhat.com/show_bug.cgi?id=1653861).
---
src/journal-remote/journal-remote-main.c | 7 +++++--
src/journal-remote/journal-remote.c | 3 +++
src/shared/journal-importer.c | 5 ++++-
3 files changed, 12 insertions(+), 3 deletions(-)
--- a/src/journal/journald-native.c
+++ b/src/journal/journald-native.c
@@ -138,6 +138,10 @@
}
/* A property follows */
+ if (n > ENTRY_FIELD_COUNT_MAX) {
+ log_debug("Received an entry that has more than " STRINGIFY(ENTRY_FIELD_COUNT_MAX) " fields, ignoring entry.");
+ goto finish;
+ }
/* n existing properties, 1 new, +1 for _TRANSPORT */
if (!GREEDY_REALLOC(iovec, m, n + 2 + N_IOVEC_META_FIELDS + N_IOVEC_OBJECT_FIELDS)) {
--- a/src/journal/journal-file.h
+++ b/src/journal/journal-file.h
@@ -160,6 +160,9 @@
* files without adding too many zeros. */
#define OFSfmt "%06"PRIx64
+/* The maximum number of fields in an entry */
+#define ENTRY_FIELD_COUNT_MAX 1024
+
static inline bool VALID_REALTIME(uint64_t u) {
/* This considers timestamps until the year 3112 valid. That should be plenty room... */
return u > 0 && u < (1ULL << 55);
--- a/src/journal-remote/journal-remote-parse.c
+++ b/src/journal-remote/journal-remote-parse.c
@@ -109,7 +109,7 @@
source->scanned = source->filled;
if (source->scanned >= DATA_SIZE_MAX) {
log_error("Entry is bigger than %u bytes.", DATA_SIZE_MAX);
- return -E2BIG;
+ return -ENOBUFS;
}
if (source->passive_fd)
--- a/src/journal-remote/journal-remote.c
+++ b/src/journal-remote/journal-remote.c
@@ -526,11 +526,18 @@
break;
else if (r < 0) {
log_warning("Failed to process data for connection %p", connection);
- if (r == -E2BIG)
+ if (r == -ENOBUFS)
return mhd_respondf(connection,
MHD_HTTP_REQUEST_ENTITY_TOO_LARGE,
"Entry is too large, maximum is %u bytes.\n",
DATA_SIZE_MAX);
+
+ else if (r == -E2BIG)
+ return mhd_respondf(connection,
+ MHD_HTTP_REQUEST_ENTITY_TOO_LARGE,
+ "Entry with more fields than the maximum of %u\n",
+ ENTRY_FIELD_COUNT_MAX);
+
else
return mhd_respondf(connection,
MHD_HTTP_UNPROCESSABLE_ENTITY,
@@ -1056,6 +1063,9 @@
log_debug("%zu active sources remaining", s->active);
return 0;
} else if (r == -E2BIG) {
+ log_notice("Entry with too many fields, skipped");
+ return 1;
+ } else if (r == -ENOBUFS) {
log_notice_errno(E2BIG, "Entry too big, skipped");
return 1;
} else if (r == -EAGAIN) {
--- a/src/journal-remote/journal-remote-write.c
+++ b/src/journal-remote/journal-remote-write.c
@@ -21,6 +21,9 @@
#include "journal-remote.h"
int iovw_put(struct iovec_wrapper *iovw, void* data, size_t len) {
+ if (iovw->count >= ENTRY_FIELD_COUNT_MAX)
+ return -E2BIG;
+
if (!GREEDY_REALLOC(iovw->iovec, iovw->size_bytes, iovw->count + 1))
return log_oom();