| From d690b5e543920906d986cc49e9a0e450e3c5b7f8 Mon Sep 17 00:00:00 2001 |
| From: Vaibhav Rustagi <vaibhavrustagi@google.com> |
| Date: Fri, 19 Jun 2020 10:42:00 -0700 |
| Subject: [PATCH] sysdeps-unix: On MSG_CTRUNC, close the fds we did receive |
| |
| MSG_CTRUNC indicates that we have received fewer fds that we should |
| have done because the buffer was too small, but we were treating it |
| as though it indicated that we received *no* fds. If we received any, |
| we still have to make sure we close them, otherwise they will be leaked. |
| |
| On the system bus, if an attacker can induce us to leak fds in this |
| way, that's a local denial of service via resource exhaustion. |
| |
| [Backport to dbus-1.10: Change signedness of iterator due to |
| commit ab8cb96e "_dbus_read_socket_with_unix_fds: make n_fds unsigned" |
| not having been applied to this branch.] |
| |
| Reported-by: Kevin Backhouse, GitHub Security Lab |
| Fixes: #294 |
| Fixes: CVE-2020-12049 |
| Fixes: GHSL-2020-057 |
| --- |
| dbus/dbus-sysdeps-unix.c | 32 ++++++++++++++++++++------------ |
| 1 file changed, 20 insertions(+), 12 deletions(-) |
| |
| diff --git a/dbus/dbus-sysdeps-unix.c b/dbus/dbus-sysdeps-unix.c |
| index ed776a70..6bba5577 100644 |
| --- a/dbus/dbus-sysdeps-unix.c |
| +++ b/dbus/dbus-sysdeps-unix.c |
| @@ -432,18 +432,6 @@ _dbus_read_socket_with_unix_fds (DBusSocket fd, |
| struct cmsghdr *cm; |
| dbus_bool_t found = FALSE; |
| |
| - if (m.msg_flags & MSG_CTRUNC) |
| - { |
| - /* Hmm, apparently the control data was truncated. The bad |
| - thing is that we might have completely lost a couple of fds |
| - without chance to recover them. Hence let's treat this as a |
| - serious error. */ |
| - |
| - errno = ENOSPC; |
| - _dbus_string_set_length (buffer, start); |
| - return -1; |
| - } |
| - |
| for (cm = CMSG_FIRSTHDR(&m); cm; cm = CMSG_NXTHDR(&m, cm)) |
| if (cm->cmsg_level == SOL_SOCKET && cm->cmsg_type == SCM_RIGHTS) |
| { |
| @@ -498,6 +486,26 @@ _dbus_read_socket_with_unix_fds (DBusSocket fd, |
| if (!found) |
| *n_fds = 0; |
| |
| + if (m.msg_flags & MSG_CTRUNC) |
| + { |
| + int i; |
| + |
| + /* Hmm, apparently the control data was truncated. The bad |
| + thing is that we might have completely lost a couple of fds |
| + without chance to recover them. Hence let's treat this as a |
| + serious error. */ |
| + |
| + /* We still need to close whatever fds we *did* receive, |
| + * otherwise they'll never get closed. (CVE-2020-12049) */ |
| + for (i = 0; i < *n_fds; i++) |
| + close (fds[i]); |
| + |
| + *n_fds = 0; |
| + errno = ENOSPC; |
| + _dbus_string_set_length (buffer, start); |
| + return -1; |
| + } |
| + |
| /* put length back (doesn't actually realloc) */ |
| _dbus_string_set_length (buffer, start + bytes_read); |
| |
| -- |
| 2.27.0.111.gc72c7da667-goog |
| |