From: Zbigniew Jędrzejewski-Szmek Date: Mon, 31 Mar 2014 02:35:37 +0000 (-0400) Subject: journal-remote: reject fields above maximum size X-Git-Tag: v216~599 X-Git-Url: https://www.chiark.greenend.org.uk/ucgi/~ianmdlvl/git?p=elogind.git;a=commitdiff_plain;h=851d4e2a67efb2c8777df151b697391ff1a76af0 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. --- diff --git a/src/journal-remote/journal-remote-parse.c b/src/journal-remote/journal-remote-parse.c index dbdf02aa3..fe21bd3e1 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 c1506d118..2b6c24ef3 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 de327cc3f..09144eaa9 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 bf02fee57..97808e746 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);