aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>2018-12-05 22:50:39 +0100
committerMike Gilbert <floppym@gentoo.org>2019-01-09 10:11:48 -0500
commit869f730a2933a3a88579278a63f3eb910ba5275d (patch)
tree01982300cfadc240e324b586527e5321bb5442b5
parentjournald: set a limit on the number of fields (1k) (diff)
downloadsystemd-869f730a2933a3a88579278a63f3eb910ba5275d.tar.gz
systemd-869f730a2933a3a88579278a63f3eb910ba5275d.tar.bz2
systemd-869f730a2933a3a88579278a63f3eb910ba5275d.zip
journald: when processing a native message, bail more quickly on overbig messages
We'd first parse all or most of the message, and only then consider if it is not too large. Also, when encountering a single field over the limit, we'd still process the preceding part of the message. Let's be stricter, and check size limits early, and let's refuse the whole message if it fails any of the size limits.
-rw-r--r--src/journal/journald-native.c65
1 files changed, 37 insertions, 28 deletions
diff --git a/src/journal/journald-native.c b/src/journal/journald-native.c
index 951d09205..110ab3641 100644
--- a/src/journal/journald-native.c
+++ b/src/journal/journald-native.c
@@ -109,7 +109,7 @@ static int server_process_entry(
int priority = LOG_INFO;
pid_t object_pid = 0;
const char *p;
- int r = 0;
+ int r = 1;
p = buffer;
@@ -121,8 +121,7 @@ static int server_process_entry(
if (!e) {
/* Trailing noise, let's ignore it, and flush what we collected */
log_debug("Received message with trailing noise, ignoring.");
- r = 1; /* finish processing of the message */
- break;
+ break; /* finish processing of the message */
}
if (e == p) {
@@ -132,8 +131,7 @@ static int server_process_entry(
}
if (IN_SET(*p, '.', '#')) {
- /* Ignore control commands for now, and
- * comments too. */
+ /* Ignore control commands for now, and comments too. */
*remaining -= (e - p) + 1;
p = e + 1;
continue;
@@ -142,7 +140,6 @@ static int server_process_entry(
/* 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.");
- r = 1;
goto finish;
}
@@ -152,7 +149,7 @@ static int server_process_entry(
N_IOVEC_META_FIELDS + N_IOVEC_OBJECT_FIELDS +
client_context_extra_fields_n_iovec(context))) {
r = log_oom();
- break;
+ goto finish;
}
q = memchr(p, '=', e - p);
@@ -161,6 +158,16 @@ static int server_process_entry(
size_t l;
l = e - p;
+ if (l > DATA_SIZE_MAX) {
+ log_debug("Received text block of %zu bytes is too large, ignoring entry.", l);
+ goto finish;
+ }
+
+ if (entry_size + l + n + 1 > ENTRY_SIZE_MAX) { /* data + separators + trailer */
+ log_debug("Entry is too big (%zu bytes after processing %zu entries), ignoring entry.",
+ entry_size + l, n + 1);
+ goto finish;
+ }
/* If the field name starts with an underscore, skip the variable, since that indicates
* a trusted field */
@@ -178,7 +185,7 @@ static int server_process_entry(
p = e + 1;
continue;
} else {
- uint64_t l;
+ uint64_t l, total;
char *k;
if (*remaining < e - p + 1 + sizeof(uint64_t) + 1) {
@@ -187,10 +194,16 @@ static int server_process_entry(
}
l = unaligned_read_le64(e + 1);
-
if (l > DATA_SIZE_MAX) {
- log_debug("Received binary data block of %"PRIu64" bytes is too large, ignoring.", l);
- break;
+ log_debug("Received binary data block of %"PRIu64" bytes is too large, ignoring entry.", l);
+ goto finish;
+ }
+
+ total = (e - p) + 1 + l;
+ if (entry_size + total + n + 1 > ENTRY_SIZE_MAX) { /* data + separators + trailer */
+ log_debug("Entry is too big (%"PRIu64"bytes after processing %zu fields), ignoring.",
+ entry_size + total, n + 1);
+ goto finish;
}
if ((uint64_t) *remaining < e - p + 1 + sizeof(uint64_t) + l + 1 ||
@@ -199,7 +212,7 @@ static int server_process_entry(
break;
}
- k = malloc((e - p) + 1 + l);
+ k = malloc(total);
if (!k) {
log_oom();
break;
@@ -228,15 +241,8 @@ static int server_process_entry(
}
}
- if (n <= 0) {
- r = 1;
+ if (n <= 0)
goto finish;
- }
-
- if (!client_context_test_priority(context, priority)) {
- r = 0;
- goto finish;
- }
tn = n++;
iovec[tn] = IOVEC_MAKE_STRING("_TRANSPORT=journal");
@@ -247,6 +253,11 @@ static int server_process_entry(
goto finish;
}
+ r = 0; /* Success, we read the message. */
+
+ if (!client_context_test_priority(context, priority))
+ goto finish;
+
if (message) {
if (s->forward_to_syslog)
server_forward_syslog(s, syslog_fixup_facility(priority), identifier, message, ucred, tv);
@@ -318,15 +329,13 @@ void server_process_native_file(
bool sealed;
int r;
- /* Data is in the passed fd, since it didn't fit in a
- * datagram. */
+ /* Data is in the passed fd, probably it didn't fit in a datagram. */
assert(s);
assert(fd >= 0);
/* If it's a memfd, check if it is sealed. If so, we can just
- * use map it and use it, and do not need to copy the data
- * out. */
+ * mmap it and use it, and do not need to copy the data out. */
sealed = memfd_get_sealed(fd) > 0;
if (!sealed && (!ucred || ucred->uid != 0)) {
@@ -397,7 +406,7 @@ void server_process_native_file(
ssize_t n;
if (fstatvfs(fd, &vfs) < 0) {
- log_error_errno(errno, "Failed to stat file system of passed file, ignoring: %m");
+ log_error_errno(errno, "Failed to stat file system of passed file, not processing it: %m");
return;
}
@@ -407,7 +416,7 @@ void server_process_native_file(
* https://github.com/systemd/systemd/issues/1822
*/
if (vfs.f_flag & ST_MANDLOCK) {
- log_error("Received file descriptor from file system with mandatory locking enabled, refusing.");
+ log_error("Received file descriptor from file system with mandatory locking enabled, not processing it.");
return;
}
@@ -420,13 +429,13 @@ void server_process_native_file(
* and so is SMB. */
r = fd_nonblock(fd, true);
if (r < 0) {
- log_error_errno(r, "Failed to make fd non-blocking, ignoring: %m");
+ log_error_errno(r, "Failed to make fd non-blocking, not processing it: %m");
return;
}
/* The file is not sealed, we can't map the file here, since
* clients might then truncate it and trigger a SIGBUS for
- * us. So let's stupidly read it */
+ * us. So let's stupidly read it. */
p = malloc(st.st_size);
if (!p) {