| From 78c1a16731f8bec0e854ed570292afc8bf3d8030 Mon Sep 17 00:00:00 2001 |
| From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl> |
| Date: Thu, 3 Jan 2019 11:35:22 +0100 |
| Subject: [PATCH] journald: do not store the iovec entry for process |
| commandline on stack |
| |
| This fixes a crash where we would read the commandline, whose length is under |
| control of the sending program, and then crash when trying to create a stack |
| allocation for it. |
| |
| CVE-2018-16864 |
| https://bugzilla.redhat.com/show_bug.cgi?id=1653855 |
| |
| The message actually doesn't get written to disk, because |
| journal_file_append_entry() returns -E2BIG. |
| |
| Resolves: #1657788 |
| |
| src/journal/coredump.c | 189 +++++++++++++--------------------- |
| src/journal/journald-server.c | 13 +-- |
| src/shared/util.c | 17 +++ |
| src/shared/util.h | 7 ++ |
| 4 files changed, 98 insertions(+), 128 deletions(-) |
| |
| diff --git a/src/journal/coredump.c b/src/journal/coredump.c |
| index 59ccd46bb0..40de86f050 100644 |
| |
| |
| @@ -526,14 +526,6 @@ static int compose_open_fds(pid_t pid, char **open_fds) { |
| } |
| |
| int main(int argc, char* argv[]) { |
| - |
| - /* The small core field we allocate on the stack, to keep things simple */ |
| - char |
| - *core_pid = NULL, *core_uid = NULL, *core_gid = NULL, *core_signal = NULL, |
| - *core_session = NULL, *core_exe = NULL, *core_comm = NULL, *core_cmdline = NULL, |
| - *core_cgroup = NULL, *core_cwd = NULL, *core_root = NULL, *core_unit = NULL, |
| - *core_slice = NULL; |
| - |
| /* The larger ones we allocate on the heap */ |
| _cleanup_free_ char |
| *core_timestamp = NULL, *core_message = NULL, *coredump_data = NULL, *core_owner_uid = NULL, |
| @@ -547,7 +539,8 @@ int main(int argc, char* argv[]) { |
| |
| struct iovec iovec[26]; |
| off_t coredump_size; |
| - int r, j = 0; |
| + int r; |
| + unsigned int n_iovec = 0; |
| uid_t uid, owner_uid; |
| gid_t gid; |
| pid_t pid; |
| @@ -634,151 +627,107 @@ int main(int argc, char* argv[]) { |
| goto finish; |
| } |
| |
| - core_unit = strjoina("COREDUMP_UNIT=", t); |
| - free(t); |
| - |
| - } else if (cg_pid_get_user_unit(pid, &t) >= 0) { |
| - core_unit = strjoina("COREDUMP_USER_UNIT=", t); |
| - free(t); |
| + if (!set_iovec_field_free(iovec, &n_iovec, "COREDUMP_UNIT=", t)) { |
| + r = log_oom(); |
| + goto finish; |
| + } |
| } |
| |
| - if (core_unit) |
| - IOVEC_SET_STRING(iovec[j++], core_unit); |
| + if (cg_pid_get_user_unit(pid, &t) >= 0) { |
| + if (!set_iovec_field_free(iovec, &n_iovec, "COREDUMP_USER_UNIT=", t)) { |
| + r = log_oom(); |
| + goto finish; |
| + } |
| + } |
| |
| /* OK, now we know it's not the journal, hence we can make use |
| * of it now. */ |
| log_set_target(LOG_TARGET_JOURNAL_OR_KMSG); |
| log_open(); |
| |
| - core_pid = strjoina("COREDUMP_PID=", info[INFO_PID]); |
| - IOVEC_SET_STRING(iovec[j++], core_pid); |
| - |
| - core_uid = strjoina("COREDUMP_UID=", info[INFO_UID]); |
| - IOVEC_SET_STRING(iovec[j++], core_uid); |
| - |
| - core_gid = strjoina("COREDUMP_GID=", info[INFO_GID]); |
| - IOVEC_SET_STRING(iovec[j++], core_gid); |
| - |
| - core_signal = strjoina("COREDUMP_SIGNAL=", info[INFO_SIGNAL]); |
| - IOVEC_SET_STRING(iovec[j++], core_signal); |
| - |
| - if (sd_pid_get_session(pid, &t) >= 0) { |
| - core_session = strjoina("COREDUMP_SESSION=", t); |
| - free(t); |
| - |
| - IOVEC_SET_STRING(iovec[j++], core_session); |
| + if (!set_iovec_string_field(iovec, &n_iovec, "COREDUMP_PID=", info[INFO_PID])) { |
| + r = log_oom(); |
| + goto finish; |
| } |
| |
| - if (sd_pid_get_owner_uid(pid, &owner_uid) >= 0) { |
| - r = asprintf(&core_owner_uid, |
| - "COREDUMP_OWNER_UID=" UID_FMT, owner_uid); |
| - if (r > 0) |
| - IOVEC_SET_STRING(iovec[j++], core_owner_uid); |
| + if (!set_iovec_string_field(iovec, &n_iovec, "COREDUMP_UID=", info[INFO_UID])) { |
| + r = log_oom(); |
| + goto finish; |
| } |
| |
| - if (sd_pid_get_slice(pid, &t) >= 0) { |
| - core_slice = strjoina("COREDUMP_SLICE=", t); |
| - free(t); |
| + if (!set_iovec_string_field(iovec, &n_iovec, "COREDUMP_GID=", info[INFO_GID])) { |
| + r = log_oom(); |
| + goto finish; |
| + } |
| |
| - IOVEC_SET_STRING(iovec[j++], core_slice); |
| + if (!set_iovec_string_field(iovec, &n_iovec, "COREDUMP_SIGNAL=", info[INFO_SIGNAL])) { |
| + r = log_oom(); |
| + goto finish; |
| } |
| |
| - if (comm) { |
| - core_comm = strjoina("COREDUMP_COMM=", comm); |
| - IOVEC_SET_STRING(iovec[j++], core_comm); |
| + if (comm && !set_iovec_string_field(iovec, &n_iovec, "COREDUMP_COMM=", comm)) { |
| + r = log_oom(); |
| + goto finish; |
| } |
| |
| - if (exe) { |
| - core_exe = strjoina("COREDUMP_EXE=", exe); |
| - IOVEC_SET_STRING(iovec[j++], core_exe); |
| + if (exe && !set_iovec_string_field(iovec, &n_iovec, "COREDUMP_EXE=", exe)) { |
| + r = log_oom(); |
| + goto finish; |
| } |
| |
| - if (get_process_cmdline(pid, 0, false, &t) >= 0) { |
| - core_cmdline = strjoina("COREDUMP_CMDLINE=", t); |
| - free(t); |
| + if (sd_pid_get_session(pid, &t) >= 0) |
| + set_iovec_field_free(iovec, &n_iovec, "COREDUMP_SESSION=", t); |
| |
| - IOVEC_SET_STRING(iovec[j++], core_cmdline); |
| + if (sd_pid_get_owner_uid(pid, &owner_uid) >= 0) { |
| + r = asprintf(&core_owner_uid, |
| + "COREDUMP_OWNER_UID=" UID_FMT, owner_uid); |
| + if (r > 0) |
| + IOVEC_SET_STRING(iovec[n_iovec++], core_owner_uid); |
| } |
| |
| - if (cg_pid_get_path_shifted(pid, NULL, &t) >= 0) { |
| - core_cgroup = strjoina("COREDUMP_CGROUP=", t); |
| - free(t); |
| + if (sd_pid_get_slice(pid, &t) >= 0) |
| + set_iovec_field_free(iovec, &n_iovec, "COREDUMP_SLICE=", t); |
| |
| - IOVEC_SET_STRING(iovec[j++], core_cgroup); |
| - } |
| + if (get_process_cmdline(pid, 0, false, &t) >= 0) |
| + set_iovec_field_free(iovec, &n_iovec, "COREDUMP_CMDLINE=", t); |
| |
| - if (compose_open_fds(pid, &t) >= 0) { |
| - core_open_fds = strappend("COREDUMP_OPEN_FDS=", t); |
| - free(t); |
| + if (cg_pid_get_path_shifted(pid, NULL, &t) >= 0) |
| + set_iovec_field_free(iovec, &n_iovec, "COREDUMP_CGROUP=", t); |
| |
| - if (core_open_fds) |
| - IOVEC_SET_STRING(iovec[j++], core_open_fds); |
| - } |
| + if (compose_open_fds(pid, &t) >= 0) |
| + set_iovec_field_free(iovec, &n_iovec, "COREDUMP_OPEN_FDS=", t); |
| |
| p = procfs_file_alloca(pid, "status"); |
| - if (read_full_file(p, &t, NULL) >= 0) { |
| - core_proc_status = strappend("COREDUMP_PROC_STATUS=", t); |
| - free(t); |
| - |
| - if (core_proc_status) |
| - IOVEC_SET_STRING(iovec[j++], core_proc_status); |
| - } |
| + if (read_full_file(p, &t, NULL) >= 0) |
| + set_iovec_field_free(iovec, &n_iovec, "COREDUMP_PROC_STATUS=", t); |
| |
| p = procfs_file_alloca(pid, "maps"); |
| - if (read_full_file(p, &t, NULL) >= 0) { |
| - core_proc_maps = strappend("COREDUMP_PROC_MAPS=", t); |
| - free(t); |
| - |
| - if (core_proc_maps) |
| - IOVEC_SET_STRING(iovec[j++], core_proc_maps); |
| - } |
| + if (read_full_file(p, &t, NULL) >= 0) |
| + set_iovec_field_free(iovec, &n_iovec, "COREDUMP_PROC_MAPS=", t); |
| |
| p = procfs_file_alloca(pid, "limits"); |
| - if (read_full_file(p, &t, NULL) >= 0) { |
| - core_proc_limits = strappend("COREDUMP_PROC_LIMITS=", t); |
| - free(t); |
| - |
| - if (core_proc_limits) |
| - IOVEC_SET_STRING(iovec[j++], core_proc_limits); |
| - } |
| + if (read_full_file(p, &t, NULL) >= 0) |
| + set_iovec_field_free(iovec, &n_iovec, "COREDUMP_PROC_LIMITS=", t); |
| |
| p = procfs_file_alloca(pid, "cgroup"); |
| - if (read_full_file(p, &t, NULL) >=0) { |
| - core_proc_cgroup = strappend("COREDUMP_PROC_CGROUP=", t); |
| - free(t); |
| + if (read_full_file(p, &t, NULL) >=0) |
| + set_iovec_field_free(iovec, &n_iovec, "COREDUMP_PROC_CGROUP=", t); |
| |
| - if (core_proc_cgroup) |
| - IOVEC_SET_STRING(iovec[j++], core_proc_cgroup); |
| - } |
| - |
| - if (get_process_cwd(pid, &t) >= 0) { |
| - core_cwd = strjoina("COREDUMP_CWD=", t); |
| - free(t); |
| + if (get_process_cwd(pid, &t) >= 0) |
| + set_iovec_field_free(iovec, &n_iovec, "COREDUMP_CWD=", t); |
| |
| - IOVEC_SET_STRING(iovec[j++], core_cwd); |
| - } |
| + if (get_process_root(pid, &t) >= 0) |
| + set_iovec_field_free(iovec, &n_iovec, "COREDUMP_ROOT=", t); |
| |
| - if (get_process_root(pid, &t) >= 0) { |
| - core_root = strjoina("COREDUMP_ROOT=", t); |
| - free(t); |
| - |
| - IOVEC_SET_STRING(iovec[j++], core_root); |
| - } |
| - |
| - if (get_process_environ(pid, &t) >= 0) { |
| - core_environ = strappend("COREDUMP_ENVIRON=", t); |
| - free(t); |
| - |
| - if (core_environ) |
| - IOVEC_SET_STRING(iovec[j++], core_environ); |
| - } |
| + if (get_process_environ(pid, &t) >= 0) |
| + set_iovec_field_free(iovec, &n_iovec, "COREDUMP_ENVIRON=", t); |
| |
| core_timestamp = strjoin("COREDUMP_TIMESTAMP=", info[INFO_TIMESTAMP], "000000", NULL); |
| if (core_timestamp) |
| - IOVEC_SET_STRING(iovec[j++], core_timestamp); |
| + IOVEC_SET_STRING(iovec[n_iovec++], core_timestamp); |
| |
| - IOVEC_SET_STRING(iovec[j++], "MESSAGE_ID=fc2e22bc6ee647b6b90729ab34a250b1"); |
| - IOVEC_SET_STRING(iovec[j++], "PRIORITY=2"); |
| + IOVEC_SET_STRING(iovec[n_iovec++], "MESSAGE_ID=fc2e22bc6ee647b6b90729ab34a250b1"); |
| + IOVEC_SET_STRING(iovec[n_iovec++], "PRIORITY=2"); |
| |
| /* Vacuum before we write anything again */ |
| coredump_vacuum(-1, arg_keep_free, arg_max_use); |
| @@ -800,7 +749,7 @@ int main(int argc, char* argv[]) { |
| const char *coredump_filename; |
| |
| coredump_filename = strjoina("COREDUMP_FILENAME=", filename); |
| - IOVEC_SET_STRING(iovec[j++], coredump_filename); |
| + IOVEC_SET_STRING(iovec[n_iovec++], coredump_filename); |
| } |
| |
| /* Vacuum again, but exclude the coredump we just created */ |
| @@ -838,7 +787,7 @@ int main(int argc, char* argv[]) { |
| log: |
| core_message = strjoin("MESSAGE=Process ", info[INFO_PID], " (", comm, ") of user ", info[INFO_UID], " dumped core.", NULL); |
| if (core_message) |
| - IOVEC_SET_STRING(iovec[j++], core_message); |
| + IOVEC_SET_STRING(iovec[n_iovec++], core_message); |
| |
| /* Optionally store the entire coredump in the journal */ |
| if (IN_SET(arg_storage, COREDUMP_STORAGE_JOURNAL, COREDUMP_STORAGE_BOTH) && |
| @@ -849,13 +798,13 @@ log: |
| |
| r = allocate_journal_field(coredump_fd, (size_t) coredump_size, &coredump_data, &sz); |
| if (r >= 0) { |
| - iovec[j].iov_base = coredump_data; |
| - iovec[j].iov_len = sz; |
| - j++; |
| + iovec[n_iovec].iov_base = coredump_data; |
| + iovec[n_iovec].iov_len = sz; |
| + n_iovec++; |
| } |
| } |
| |
| - r = sd_journal_sendv(iovec, j); |
| + r = sd_journal_sendv(iovec, n_iovec); |
| if (r < 0) |
| log_error_errno(r, "Failed to log coredump: %m"); |
| |
| diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c |
| index 7e67e055e3..c35858247b 100644 |
| |
| |
| @@ -788,9 +788,9 @@ static void dispatch_message_real( |
| |
| r = get_process_cmdline(ucred->pid, 0, false, &t); |
| if (r >= 0) { |
| - x = strjoina("_CMDLINE=", t); |
| - free(t); |
| - IOVEC_SET_STRING(iovec[n++], x); |
| + /* At most _SC_ARG_MAX (2MB usually), which is too much to put on stack. |
| + * Let's use a heap allocation for this one. */ |
| + set_iovec_field_free(iovec, &n, "_CMDLINE=", t); |
| } |
| |
| r = get_process_capeff(ucred->pid, &t); |
| @@ -915,11 +915,8 @@ static void dispatch_message_real( |
| } |
| |
| r = get_process_cmdline(object_pid, 0, false, &t); |
| - if (r >= 0) { |
| - x = strjoina("OBJECT_CMDLINE=", t); |
| - free(t); |
| - IOVEC_SET_STRING(iovec[n++], x); |
| - } |
| + if (r >= 0) |
| + set_iovec_field_free(iovec, &n, "OBJECT_CMDLINE=", t); |
| |
| #ifdef HAVE_AUDIT |
| r = audit_session_from_pid(object_pid, &audit); |
| diff --git a/src/shared/util.c b/src/shared/util.c |
| index 78967103a6..c71e021cd7 100644 |
| |
| |
| @@ -2275,6 +2275,23 @@ int flush_fd(int fd) { |
| } |
| } |
| |
| +char* set_iovec_string_field(struct iovec *iovec, unsigned int *n_iovec, const char *field, const char *value) { |
| + char *x; |
| + |
| + x = strappend(field, value); |
| + if (x) |
| + iovec[(*n_iovec)++] = IOVEC_MAKE_STRING(x); |
| + return x; |
| +} |
| + |
| +char* set_iovec_field_free(struct iovec *iovec, unsigned int *n_iovec, const char *field, char *value) { |
| + char *x; |
| + |
| + x = set_iovec_string_field(iovec, n_iovec, field, value); |
| + free(value); |
| + return x; |
| +} |
| + |
| int acquire_terminal( |
| const char *name, |
| bool fail, |
| diff --git a/src/shared/util.h b/src/shared/util.h |
| index cf096aa07b..8fc237495a 100644 |
| |
| |
| @@ -1140,3 +1140,10 @@ static inline void block_signals_reset(sigset_t *ss) { |
| _t; \ |
| }) |
| |
| +#define IOVEC_INIT(base, len) { .iov_base = (base), .iov_len = (len) } |
| +#define IOVEC_MAKE(base, len) (struct iovec) IOVEC_INIT(base, len) |
| +#define IOVEC_INIT_STRING(string) IOVEC_INIT((char*) string, strlen(string)) |
| +#define IOVEC_MAKE_STRING(string) (struct iovec) IOVEC_INIT_STRING(string) |
| + |
| +char* set_iovec_string_field(struct iovec *iovec, unsigned int *n_iovec, const char *field, const char *value); |
| +char* set_iovec_field_free(struct iovec *iovec, unsigned int *n_iovec, const char *field, char *value); |