journal-remote: reject fields above maximum size

Also fix an infinite loop on E2BIG.

Remember what range we already scanned for '\n', to avoid
quadratic behaviour on long "text" fields.

4 files changed, 55 insertions, 44 deletions

diff --git a/src/journal-remote/journal-remote-parse.c b/src/journal-remote/journal-remote-parse.c
index dbdf02aa3c..fe21bd3e1d 100644
--- a/src/journal-remote/journal-remote-parse.c
+++ b/src/journal-remote/journal-remote-parse.c
@@ -49,43 +49,43 @@ static int get_line(RemoteSource *source, char **line, size_t *size) {
         assert(source->filled <= source->size);
         assert(source->buf == NULL || source->size > 0);
 
-        if (source->buf)
-                c = memchr(source->buf, '\n', source->filled);
-
-        if (c != NULL)
-                goto docopy;
-
- resize:
-        if (source->fd < 0)
-                /* we have to wait for some data to come to us */
-                return -EWOULDBLOCK;
+        while (true) {
+                if (source->buf)
+                        c = memchr(source->buf + source->scanned, '\n',
+                                   source->filled - source->scanned);
+                if (c != NULL)
+                        break;
+
+                source->scanned = source->filled;
+                if (source->scanned >= DATA_SIZE_MAX) {
+                        log_error("Entry is bigger than %u bytes.", DATA_SIZE_MAX);
+                        return -E2BIG;
+                }
 
-        if (source->size - source->filled < LINE_CHUNK) {
-                // XXX: add check for maximum line length
+                if (source->fd < 0)
+                        /* we have to wait for some data to come to us */
+                        return -EWOULDBLOCK;
 
-                if (!GREEDY_REALLOC(source->buf, source->size,
-                                    source->filled + LINE_CHUNK))
-                        return log_oom();
-        }
-        assert(source->size - source->filled >= LINE_CHUNK);
+                if (source->size - source->filled < LINE_CHUNK &&
+                    !GREEDY_REALLOC(source->buf, source->size,
+                                    MAX(source->filled + LINE_CHUNK, DATA_SIZE_MAX)))
+                                return log_oom();
 
-        n = read(source->fd, source->buf + source->filled,
-                 source->size - source->filled);
-        if (n < 0) {
-                if (errno != EAGAIN && errno != EWOULDBLOCK)
-                        log_error("read(%d, ..., %zd): %m", source->fd,
-                                  source->size - source->filled);
-                return -errno;
-        } else if (n == 0)
-                return 0;
+                assert(source->size - source->filled >= LINE_CHUNK);
 
-        c = memchr(source->buf + source->filled, '\n', n);
-        source->filled += n;
+                n = read(source->fd, source->buf + source->filled,
+                         MAX(source->size, DATA_SIZE_MAX) - source->filled);
+                if (n < 0) {
+                        if (errno != EAGAIN && errno != EWOULDBLOCK)
+                                log_error("read(%d, ..., %zd): %m", source->fd,
+                                          source->size - source->filled);
+                        return -errno;
+                } else if (n == 0)
+                        return 0;
 
-        if (c == NULL)
-                goto resize;
+                source->filled += n;
+        }
 
- docopy:
         *line = source->buf;
         *size = c + 1 - source->buf;
 
@@ -102,6 +102,7 @@ static int get_line(RemoteSource *source, char **line, size_t *size) {
         source->buf = newbuf;
         source->size = newsize;
         source->filled = remain;
+        source->scanned = 0;
 
         return 1;
 }
@@ -111,8 +112,12 @@ int push_data(RemoteSource *source, const char *data, size_t size) {
         assert(source->state != STATE_EOF);
 
         if (!GREEDY_REALLOC(source->buf, source->size,
-                            source->filled + size))
-                return log_oom();
+                            source->filled + size)) {
+                log_error("Failed to store received data of size %zu "
+                          "(in addition to existing %zu bytes with %zu filled): %s",
+                          size, source->size, source->filled, strerror(ENOMEM));
+                return -ENOMEM;
+        }
 
         memcpy(source->buf + source->filled, data, size);
         source->filled += size;
@@ -131,6 +136,7 @@ static int fill_fixed_size(RemoteSource *source, void **data, size_t size) {
                source->state == STATE_DATA_FINISH);
         assert(size <= DATA_SIZE_MAX);
         assert(source->filled <= source->size);
+        assert(source->scanned <= source->filled);
         assert(source->buf != NULL || source->size == 0);
         assert(source->buf == NULL || source->size > 0);
         assert(data);
@@ -171,6 +177,7 @@ static int fill_fixed_size(RemoteSource *source, void **data, size_t size) {
         source->buf = newbuf;
         source->size = newsize;
         source->filled = remain;
+        source->scanned = 0;
 
         return 1;
 }
diff --git a/src/journal-remote/journal-remote-parse.h b/src/journal-remote/journal-remote-parse.h
index c1506d118d..2b6c24ef3a 100644
--- a/src/journal-remote/journal-remote-parse.h
+++ b/src/journal-remote/journal-remote-parse.h
@@ -38,6 +38,7 @@ typedef struct RemoteSource {
 
         char *buf;
         size_t size;
+        size_t scanned;
         size_t filled;
         size_t data_size;
 
diff --git a/src/journal-remote/journal-remote.c b/src/journal-remote/journal-remote.c
index de327cc3fd..09144eaa97 100644
--- a/src/journal-remote/journal-remote.c
+++ b/src/journal-remote/journal-remote.c
@@ -412,25 +412,28 @@ static int process_http_upload(
                 log_info("Received %zu bytes", *upload_data_size);
 
                 r = push_data(source, upload_data, *upload_data_size);
-                if (r < 0) {
-                        log_error("Failed to store received data of size %zu: %s",
-                                  *upload_data_size, strerror(-r));
+                if (r < 0)
                         return mhd_respond_oom(connection);
-                }
+
                 *upload_data_size = 0;
         } else
                 finished = true;
 
         while (true) {
                 r = process_source(source, &server->writer, arg_compress, arg_seal);
-                if (r == -E2BIG)
-                        log_warning("Entry too big, skipped");
-                else if (r == -EAGAIN || r == -EWOULDBLOCK)
+                if (r == -EAGAIN || r == -EWOULDBLOCK)
                         break;
                 else if (r < 0) {
                         log_warning("Failed to process data for connection %p", connection);
-                        return mhd_respondf(connection, MHD_HTTP_UNPROCESSABLE_ENTITY,
-                                            "Processing failed: %s", strerror(-r));
+                        if (r == -E2BIG)
+                                return mhd_respondf(connection,
+                                                    MHD_HTTP_REQUEST_ENTITY_TOO_LARGE,
+                                                    "Entry is too large, maximum is %u bytes.\n",
+                                                    DATA_SIZE_MAX);
+                        else
+                                return mhd_respondf(connection,
+                                                    MHD_HTTP_UNPROCESSABLE_ENTITY,
+                                                    "Processing failed: %s.", strerror(-r));
                 }
         }
 
diff --git a/src/journal/journald-native.h b/src/journal/journald-native.h
index bf02fee576..97808e746a 100644
--- a/src/journal/journald-native.h
+++ b/src/journal/journald-native.h
@@ -25,8 +25,8 @@
 
 /* Make sure not to make this smaller than the maximum coredump
  * size. See COREDUMP_MAX in coredump.c */
-#define ENTRY_SIZE_MAX (1024*1024*768)
-#define DATA_SIZE_MAX (1024*1024*768)
+#define ENTRY_SIZE_MAX (1024*1024*768u)
+#define DATA_SIZE_MAX (1024*1024*768u)
 
 bool valid_user_field(const char *p, size_t l, bool allow_protected);