journal: check validity of PID field in syslog/kmsg earlier

Then, let's make syslog_parse_identifier() returns PID as pid_t, rather
than the string.

This also makes the function refuse ridiculously long identifier.

Fixes #40456.
Fixes oss-fuzz#477990732 (https://issues.oss-fuzz.com/issues/477990732).

(cherry picked from commit eb5b797d7c)
This commit is contained in:
Yu Watanabe
2026-01-28 13:23:50 +09:00
committed by Luca Boccassi
parent adc0290a9d
commit 9fa74ea900
5 changed files with 67 additions and 51 deletions

View File

@@ -85,28 +85,23 @@ void manager_forward_kmsg(
log_debug_errno(errno, "Failed to write to /dev/kmsg for logging, ignoring: %m");
}
static bool is_us(const char *identifier, const char *pid) {
pid_t pid_num;
if (!identifier || !pid)
static bool is_us(const char *identifier, pid_t pid) {
if (!identifier || !pid_is_valid(pid))
return false;
if (parse_pid(pid, &pid_num) < 0)
return false;
return pid_num == getpid_cached() &&
return pid == getpid_cached() &&
streq(identifier, program_invocation_short_name);
}
void dev_kmsg_record(Manager *m, char *p, size_t l) {
_cleanup_free_ char *message = NULL, *syslog_pid = NULL, *syslog_identifier = NULL, *identifier = NULL, *pid = NULL;
_cleanup_free_ char *message = NULL, *syslog_identifier = NULL;
struct iovec iovec[N_IOVEC_META_FIELDS + 7 + N_IOVEC_KERNEL_FIELDS + 2 + N_IOVEC_UDEV_FIELDS];
char *kernel_device = NULL;
unsigned long long usec;
size_t n = 0, z = 0, j;
int priority, r;
char *e, *k;
char *e, *k, syslog_pid[STRLEN("SYSLOG_PID=") + DECIMAL_STR_MAX(pid_t)];
uint64_t serial;
size_t pl;
int saved_log_max_level = INT_MAX;
@@ -284,6 +279,9 @@ void dev_kmsg_record(Manager *m, char *p, size_t l) {
if (LOG_FAC(priority) == LOG_KERN)
iovec[n++] = IOVEC_MAKE_STRING("SYSLOG_IDENTIFIER=kernel");
else {
_cleanup_free_ char *identifier = NULL;
pid_t pid;
pl -= syslog_parse_identifier((const char**) &p, &identifier, &pid);
/* Avoid logging any new messages when we're processing messages generated by ourselves via
@@ -303,10 +301,9 @@ void dev_kmsg_record(Manager *m, char *p, size_t l) {
iovec[n++] = IOVEC_MAKE_STRING(syslog_identifier);
}
if (pid) {
syslog_pid = strjoin("SYSLOG_PID=", pid);
if (syslog_pid)
iovec[n++] = IOVEC_MAKE_STRING(syslog_pid);
if (pid_is_valid(pid)) {
xsprintf(syslog_pid, "SYSLOG_PID="PID_FMT, pid);
iovec[n++] = IOVEC_MAKE_STRING(syslog_pid);
}
}

View File

@@ -21,6 +21,7 @@
#include "journald-wall.h"
#include "log.h"
#include "log-ratelimit.h"
#include "parse-util.h"
#include "process-util.h"
#include "selinux-util.h"
#include "socket-util.h"
@@ -200,14 +201,14 @@ int syslog_fixup_facility(int priority) {
return priority;
}
size_t syslog_parse_identifier(const char **buf, char **identifier, char **pid) {
size_t syslog_parse_identifier(const char **buf, char **ret_identifier, pid_t *ret_pid) {
const char *p;
char *t;
size_t l, e;
pid_t pid = 0;
assert(buf);
assert(identifier);
assert(pid);
assert(ret_identifier);
assert(ret_pid);
p = *buf;
@@ -215,8 +216,11 @@ size_t syslog_parse_identifier(const char **buf, char **identifier, char **pid)
l = strcspn(p, WHITESPACE);
if (l <= 0 ||
p[l-1] != ':')
p[l-1] != ':') {
*ret_identifier = NULL;
*ret_pid = 0;
return 0;
}
e = l;
l--;
@@ -227,9 +231,9 @@ size_t syslog_parse_identifier(const char **buf, char **identifier, char **pid)
for (;;) {
if (p[k] == '[') {
t = strndup(p+k+1, l-k-2);
_cleanup_free_ char *t = strndup(p+k+1, l-k-2);
if (t)
*pid = t;
(void) parse_pid(t, &pid);
l = k;
break;
@@ -242,9 +246,13 @@ size_t syslog_parse_identifier(const char **buf, char **identifier, char **pid)
}
}
t = strndup(p, l);
if (t)
*identifier = t;
/* The syslog identifier should be short enough in most cases and NAME_MAX should be enough. Let's
* refuse ridiculously long identifier string as "no identifier string found", because if it is
* longer than some threshold then it is quite likely some misformatted data, and not a valid syslog
* message. Note. NAME_MAX is counted *without* the trailing NUL. */
_cleanup_free_ char *identifier = NULL;
if (l <= NAME_MAX)
identifier = strndup(p, l); /* ignore OOM here. */
/* Single space is used as separator */
if (p[e] != '\0' && strchr(WHITESPACE, p[e]))
@@ -252,6 +260,8 @@ size_t syslog_parse_identifier(const char **buf, char **identifier, char **pid)
l = (p - *buf) + e;
*buf = p + e;
*ret_identifier = TAKE_PTR(identifier);
*ret_pid = pid;
return l;
}
@@ -335,8 +345,7 @@ void manager_process_syslog_message(
char *t, syslog_priority[STRLEN("PRIORITY=") + DECIMAL_STR_MAX(int)],
syslog_facility[STRLEN("SYSLOG_FACILITY=") + DECIMAL_STR_MAX(int)];
const char *msg, *syslog_ts, *a;
_cleanup_free_ char *identifier = NULL, *pid = NULL,
*dummy = NULL, *msg_msg = NULL, *msg_raw = NULL;
_cleanup_free_ char *dummy = NULL, *msg_msg = NULL, *msg_raw = NULL;
int priority = LOG_USER | LOG_INFO, r;
ClientContext *context = NULL;
struct iovec *iovec;
@@ -399,6 +408,8 @@ void manager_process_syslog_message(
/* We failed to parse the full timestamp, store the raw message too */
store_raw = true;
_cleanup_free_ char *identifier = NULL;
pid_t pid;
syslog_parse_identifier(&msg, &identifier, &pid);
if (client_context_check_keep_log(context, msg, strlen(msg)) <= 0)
@@ -434,9 +445,10 @@ void manager_process_syslog_message(
iovec[n++] = IOVEC_MAKE_STRING(a);
}
if (pid) {
a = strjoina("SYSLOG_PID=", pid);
iovec[n++] = IOVEC_MAKE_STRING(a);
char syslog_pid[STRLEN("SYSLOG_PID=") + DECIMAL_STR_MAX(pid_t)];
if (pid_is_valid(pid)) {
xsprintf(syslog_pid, "SYSLOG_PID="PID_FMT, pid);
iovec[n++] = IOVEC_MAKE_STRING(syslog_pid);
}
if (syslog_ts_len > 0) {

View File

@@ -5,7 +5,7 @@
int syslog_fixup_facility(int priority) _const_;
size_t syslog_parse_identifier(const char **buf, char **identifier, char **pid);
size_t syslog_parse_identifier(const char **buf, char **ret_identifier, pid_t *ret_pid);
void manager_forward_syslog(Manager *m, int priority, const char *identifier, const char *message, const struct ucred *ucred, const struct timeval *tv);

View File

@@ -5,18 +5,24 @@
#include "syslog-util.h"
#include "tests.h"
static void test_syslog_parse_identifier_one(const char *str,
const char *ident, const char *pid, const char *rest, int ret) {
static void test_syslog_parse_identifier_one(
const char *str,
const char *ident,
pid_t pid,
const char *rest,
int ret) {
const char *buf = str;
_cleanup_free_ char *ident2 = NULL, *pid2 = NULL;
_cleanup_free_ char *ident2 = NULL;
pid_t pid2;
int ret2;
ret2 = syslog_parse_identifier(&buf, &ident2, &pid2);
assert_se(ret == ret2);
assert_se(ident == ident2 || streq_ptr(ident, ident2));
assert_se(pid == pid2 || streq_ptr(pid, pid2));
assert_se(streq(buf, rest));
ASSERT_EQ(ret, ret2);
ASSERT_STREQ(ident, ident2);
ASSERT_EQ(pid, pid2);
ASSERT_STREQ(buf, rest);
}
static void test_syslog_parse_priority_one(const char *str, bool with_facility, int priority, int ret) {
@@ -30,20 +36,20 @@ static void test_syslog_parse_priority_one(const char *str, bool with_facility,
}
TEST(syslog_parse_identifier) {
test_syslog_parse_identifier_one("pidu[111]: xxx", "pidu", "111", "xxx", 11);
test_syslog_parse_identifier_one("pidu: xxx", "pidu", NULL, "xxx", 6);
test_syslog_parse_identifier_one("pidu: xxx", "pidu", NULL, " xxx", 6);
test_syslog_parse_identifier_one("pidu xxx", NULL, NULL, "pidu xxx", 0);
test_syslog_parse_identifier_one(" pidu xxx", NULL, NULL, " pidu xxx", 0);
test_syslog_parse_identifier_one("", NULL, NULL, "", 0);
test_syslog_parse_identifier_one(" ", NULL, NULL, " ", 0);
test_syslog_parse_identifier_one(":", "", NULL, "", 1);
test_syslog_parse_identifier_one(": ", "", NULL, " ", 2);
test_syslog_parse_identifier_one(" :", "", NULL, "", 2);
test_syslog_parse_identifier_one(" pidu:", "pidu", NULL, "", 8);
test_syslog_parse_identifier_one("pidu:", "pidu", NULL, "", 5);
test_syslog_parse_identifier_one("pidu: ", "pidu", NULL, "", 6);
test_syslog_parse_identifier_one("pidu : ", NULL, NULL, "pidu : ", 0);
test_syslog_parse_identifier_one("pidu[111]: xxx", "pidu", 111, "xxx", 11);
test_syslog_parse_identifier_one("pidu: xxx", "pidu", 0, "xxx", 6);
test_syslog_parse_identifier_one("pidu: xxx", "pidu", 0, " xxx", 6);
test_syslog_parse_identifier_one("pidu xxx", NULL, 0, "pidu xxx", 0);
test_syslog_parse_identifier_one(" pidu xxx", NULL, 0, " pidu xxx", 0);
test_syslog_parse_identifier_one("", NULL, 0, "", 0);
test_syslog_parse_identifier_one(" ", NULL, 0, " ", 0);
test_syslog_parse_identifier_one(":", "", 0, "", 1);
test_syslog_parse_identifier_one(": ", "", 0, " ", 2);
test_syslog_parse_identifier_one(" :", "", 0, "", 2);
test_syslog_parse_identifier_one(" pidu:", "pidu", 0, "", 8);
test_syslog_parse_identifier_one("pidu:", "pidu", 0, "", 5);
test_syslog_parse_identifier_one("pidu: ", "pidu", 0, "", 6);
test_syslog_parse_identifier_one("pidu : ", NULL, 0, "pidu : ", 0);
}
TEST(syslog_parse_priority) {

File diff suppressed because one or more lines are too long