SymbX/systemd
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

journald-remote/gateway hardening fixes flagged by kres (#42733)

Browse Source
This commit is contained in:
Lennart Poettering
2026-06-25 22:09:58 +02:00
committed by GitHub
parent 46ece03c99 94d49182a0
commit 2da8cd09c2
5 changed files with 48 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/journal-remote/journal-gatewayd.c
+11 -3
View File
@@ -184,9 +184,12 @@ static ssize_t request_reader_entries(
m->n_entries <= 0)
return MHD_CONTENT_READER_END_OF_STREAM;
if (m->n_skip < 0)
if (m->n_skip < 0) {
/* request_parse_range_skip_and_n_entries() rejects INT64_MIN, so the negation
* below cannot overflow. */
assert(m->n_skip >= -INT64_MAX);
r = sd_journal_previous_skip(m->journal, (uint64_t) -m->n_skip + 1);
else if (m->n_skip > 0) {
} else if (m->n_skip > 0) {
r = sd_journal_next_skip(m->journal, (uint64_t) m->n_skip + 1);
if (r < 0) {
log_error_errno(r, "Failed to skip journal entries: %m");
@@ -194,7 +197,7 @@ static ssize_t request_reader_entries(
}
/* We skipped beyond the end, make sure entries between the cursor and n_skip offset
* from it are not returned. */
if (r < m->n_skip + 1) {
if ((uint64_t) r < (uint64_t) m->n_skip + 1) {
m->n_skip -= r;
if (!m->follow)
@@ -348,6 +351,11 @@ static int request_parse_range_skip_and_n_entries(
r = safe_atoi64(t, &m->n_skip);
if (r < 0)
return r;
/* The consumer negates n_skip, and negating INT64_MIN in signed arithmetic is undefined
* behaviour, so reject it up front. */
if (m->n_skip == INT64_MIN)
return -ERANGE;
}
p = (colon2 ?: colon) + 1;
src/journal-remote/journal-remote-main.c
+7 -6
View File
@@ -221,21 +221,23 @@ static int build_accept_encoding(char **ret) {
return 0;
}
static int request_meta(void **connection_cls, int fd, char *hostname) {
static int request_meta(void **connection_cls, int fd, char *_hostname) {
int r;
assert(connection_cls);
/* This takes ownership of the hostname in all cases, including on failure. */
_cleanup_free_ char *hostname = TAKE_PTR(_hostname);
if (*connection_cls)
return 0; /* already assigned. */
Writer *writer;
r = journal_remote_get_writer(journal_remote_server_global, hostname, &writer);
if (r < 0)
return log_warning_errno(r, "Failed to get writer for source %s: %m",
hostname);
return log_warning_errno(r, "Failed to get writer for source %s: %m", hostname);
_cleanup_(source_freep) RemoteSource *source = source_new(fd, true, hostname, writer);
_cleanup_(source_freep) RemoteSource *source = source_new(fd, true, TAKE_PTR(hostname), writer);
if (!source)
return log_oom();
@@ -445,13 +447,12 @@ static mhd_result request_handler(
assert(hostname);
r = request_meta(connection_cls, fd, hostname);
r = request_meta(connection_cls, fd, TAKE_PTR(hostname));
if (r == -ENOMEM)
return respond_oom(connection);
else if (r < 0)
return mhd_respondf(connection, r, MHD_HTTP_INTERNAL_SERVER_ERROR, "%m");
hostname = NULL;
return MHD_YES;
}
src/journal-remote/journal-remote-parse.c
+5 -2
View File
@@ -24,7 +24,8 @@ RemoteSource* source_free(RemoteSource *source) {
/**
* Initialize zero-filled source with given values. On success, takes
* ownership of fd, name, and writer, otherwise does not touch them.
* ownership of fd and writer, otherwise does not touch them. Always takes
* ownership of name, even on failure.
*/
RemoteSource* source_new(int fd, bool passive_fd, char *name, Writer *writer) {
RemoteSource *source;
@@ -35,8 +36,10 @@ RemoteSource* source_new(int fd, bool passive_fd, char *name, Writer *writer) {
assert(fd >= 0);
source = new0(RemoteSource, 1);
if (!source)
if (!source) {
free(name);
return NULL;
}
source->importer = JOURNAL_IMPORTER_MAKE(fd);
source->importer.passive_fd = passive_fd;
src/journal-remote/journal-remote.c
+19 -19
View File
@@ -162,11 +162,12 @@ static int dispatch_raw_connection_event(sd_event_source *event,
void *userdata);
static int get_source_for_fd(RemoteServer *s,
int fd, char *name, RemoteSource **source) {
int fd, char *_name, RemoteSource **source) {
_cleanup_free_ char *name = TAKE_PTR(_name);
Writer *writer;
int r;
/* This takes ownership of name, but only on success. */
/* This takes ownership of the name in all cases, including on failure. */
assert(s);
assert(fd >= 0);
@@ -177,11 +178,10 @@ static int get_source_for_fd(RemoteServer *s,
r = journal_remote_get_writer(s, name, &writer);
if (r < 0)
return log_warning_errno(r, "Failed to get writer for source %s: %m",
name);
return log_warning_errno(r, "Failed to get writer for source %s: %m", name);
if (!s->sources[fd]) {
s->sources[fd] = source_new(fd, false, name, writer);
s->sources[fd] = source_new(fd, false, TAKE_PTR(name), writer);
if (!s->sources[fd]) {
writer_unref(writer);
return log_oom();
@@ -211,29 +211,29 @@ static int remove_source(RemoteServer *s, int fd) {
return 0;
}
int journal_remote_add_source(RemoteServer *s, int fd, char *name, bool own_name) {
int journal_remote_add_source(RemoteServer *s, int fd, char *_name, bool own_name) {
_cleanup_free_ char *name = NULL;
RemoteSource *source = NULL;
int r;
/* This takes ownership of name, even on failure, if own_name is true. */
/* This takes ownership of _name, even on failure, if own_name is true. */
assert(s);
assert(fd >= 0);
assert(name);
assert(_name);
if (!own_name) {
name = strdup(name);
if (own_name)
name = TAKE_PTR(_name);
else {
name = strdup(_name);
if (!name)
return log_oom();
}
r = get_source_for_fd(s, fd, name, &source);
if (r < 0) {
log_error_errno(r, "Failed to create source for fd:%d (%s): %m",
fd, name);
free(name);
return r;
}
/* get_source_for_fd() takes ownership of the name in all cases, so it must not be touched below. */
r = get_source_for_fd(s, fd, TAKE_PTR(name), &source);
if (r < 0)
return log_error_errno(r, "Failed to create source for fd:%d: %m", fd);
r = sd_event_add_io(s->event, &source->event,
fd, EPOLLIN|EPOLLRDHUP|EPOLLPRI,
@@ -246,7 +246,7 @@ int journal_remote_add_source(RemoteServer *s, int fd, char *name, bool own_name
if (r == 0)
r = sd_event_source_set_enabled(source->buffer_event, SD_EVENT_OFF);
} else if (r == -EPERM) {
log_debug("Falling back to sd_event_add_defer for fd:%d (%s)", fd, name);
log_debug("Falling back to sd_event_add_defer for fd:%d (%s)", fd, source->importer.name);
r = sd_event_add_defer(s->event, &source->event,
dispatch_blocking_source_event, source);
if (r == 0)
@@ -258,7 +258,7 @@ int journal_remote_add_source(RemoteServer *s, int fd, char *name, bool own_name
goto error;
}
r = sd_event_source_set_description(source->event, name);
r = sd_event_source_set_description(source->event, source->importer.name);
if (r < 0) {
log_error_errno(r, "Failed to set source name for fd:%d: %m", fd);
goto error;
test/units/TEST-04-JOURNAL.journal-gatewayd.sh
+6
View File
@@ -88,6 +88,12 @@ curl -LSfs \
--header "Range: entries=$TEST_CURSOR:1:1" \
http://localhost:19531/entries?SYSLOG_IDENTIFIER="$TEST_TAG" >"$LOG_FILE"
jq -se "length == 0" "$LOG_FILE"
# A skip of INT64_MIN must not crash the daemon
curl -LSs \
--header "Accept: application/json" \
--header "Range: entries=:-9223372036854775808:1" \
http://localhost:19531/entries >/dev/null || true
curl -LSfs http://localhost:19531/entries >"$LOG_FILE"
# Check if the specified cursor refers to an existing entry and return just that entry
curl -LSfs \
--header "Accept: application/json" \