diff --git a/SOURCES/flatpak-0.8.8-cve-2018-6560.patch b/SOURCES/flatpak-0.8.8-cve-2018-6560.patch new file mode 100644 index 0000000..1125a7a --- /dev/null +++ b/SOURCES/flatpak-0.8.8-cve-2018-6560.patch @@ -0,0 +1,214 @@ +From e00ded769dcdddea0169dd813c5878c915a63f6a Mon Sep 17 00:00:00 2001 +From: Alexander Larsson +Date: Sun, 28 Jan 2018 20:51:54 +0100 +Subject: [PATCH] Fix vulnerability in dbus proxy + +During the authentication all client data is directly forwarded +to the dbus daemon as is, until we detect the BEGIN command after +which we start filtering the binary dbus protocol. + +Unfortunately the detection of the BEGIN command in the proxy +did not exactly match the detection in the dbus daemon. A BEGIN +followed by a space or tab was considered ok in the daemon but +not by the proxy. This could be exploited to send arbitrary +dbus messages to the host, which can be used to break out of +the sandbox. + +This was noticed by Gabriel Campana of The Google Security Team. + +This fix makes the detection of the authentication phase end +match the dbus code. In addition we duplicate the authentication +line validation from dbus, which includes ensuring all data is +ASCII, and limiting the size of a line to 16k. In fact, we add +some extra stringent checks, disallowing ASCII control chars and +requiring that auth lines start with a capital letter. +--- + dbus-proxy/flatpak-proxy.c | 127 ++++++++++++++++++++++++++----------- + 1 file changed, 89 insertions(+), 38 deletions(-) + +diff --git a/dbus-proxy/flatpak-proxy.c b/dbus-proxy/flatpak-proxy.c +index aee73993..ec90cba7 100644 +--- a/dbus-proxy/flatpak-proxy.c ++++ b/dbus-proxy/flatpak-proxy.c +@@ -173,10 +173,11 @@ typedef struct FlatpakProxyClient FlatpakProxyClient; + FlatpakPolicy flatpak_proxy_get_policy (FlatpakProxy *proxy, + const char *name); + +-/* We start looking ignoring the first cr-lf +- since there is no previous line initially */ +-#define AUTH_END_INIT_OFFSET 2 +-#define AUTH_END_STRING "\r\nBEGIN\r\n" ++#define FIND_AUTH_END_CONTINUE -1 ++#define FIND_AUTH_END_ABORT -2 ++ ++#define AUTH_LINE_SENTINEL "\r\n" ++#define AUTH_BEGIN "BEGIN" + + typedef enum { + EXPECTED_REPLY_NONE, +@@ -251,7 +252,7 @@ struct FlatpakProxyClient + FlatpakProxy *proxy; + + gboolean authenticated; +- int auth_end_offset; ++ GByteArray *auth_buffer; + + ProxySide client_side; + ProxySide bus_side; +@@ -363,6 +364,7 @@ flatpak_proxy_client_finalize (GObject *object) + client->proxy->clients = g_list_remove (client->proxy->clients, client); + g_clear_object (&client->proxy); + ++ g_byte_array_free (client->auth_buffer, TRUE); + g_hash_table_destroy (client->rewrite_reply); + g_hash_table_destroy (client->get_owner_reply); + g_hash_table_destroy (client->unique_id_policy); +@@ -398,7 +400,7 @@ flatpak_proxy_client_init (FlatpakProxyClient *client) + init_side (client, &client->client_side); + init_side (client, &client->bus_side); + +- client->auth_end_offset = AUTH_END_INIT_OFFSET; ++ client->auth_buffer = g_byte_array_new (); + client->rewrite_reply = g_hash_table_new_full (g_direct_hash, g_direct_equal, NULL, g_object_unref); + client->get_owner_reply = g_hash_table_new_full (g_direct_hash, g_direct_equal, NULL, g_free); + client->unique_id_policy = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL); +@@ -2203,51 +2205,92 @@ got_buffer_from_side (ProxySide *side, Buffer *buffer) + got_buffer_from_bus (client, side, buffer); + } + ++#define _DBUS_ISASCII(c) ((c) != '\0' && (((c) & ~0x7f) == 0)) ++ ++static gboolean ++auth_line_is_valid (guint8 *line, guint8 *line_end) ++{ ++ guint8 *p; ++ ++ for (p = line; p < line_end; p++) ++ { ++ if (!_DBUS_ISASCII(*p)) ++ return FALSE; ++ ++ /* Technically, the dbus spec allows all ASCII characters, but for robustness we also ++ fail if we see any control characters. Such low values will appear in potential attacks, ++ but will never happen in real sasl (where all binary data is hex encoded). */ ++ if (*p < ' ') ++ return FALSE; ++ } ++ ++ /* For robustness we require the first char of the line to be an upper case letter. ++ This is not technically required by the dbus spec, but all commands are upper ++ case, and there is no provisioning for whitespace before the command, so in practice ++ this is true, and this means we're not confused by e.g. initial whitespace. */ ++ if (line[0] < 'A' || line[0] > 'Z') ++ return FALSE; ++ ++ return TRUE; ++} ++ ++static gboolean ++auth_line_is_begin (guint8 *line) ++{ ++ guint8 next_char; ++ ++ if (!g_str_has_prefix ((char *)line, AUTH_BEGIN)) ++ return FALSE; ++ ++ /* dbus-daemon accepts either nothing, or a whitespace followed by anything as end of auth */ ++ next_char = line[strlen (AUTH_BEGIN)]; ++ return (next_char == 0 || ++ next_char == ' ' || ++ next_char == '\t'); ++} ++ + static gssize + find_auth_end (FlatpakProxyClient *client, Buffer *buffer) + { +- guchar *match; +- int i; ++ goffset offset = 0; ++ gsize original_size = client->auth_buffer->len; ++ ++ /* Add the new data to the remaining data from last iteration */ ++ g_byte_array_append (client->auth_buffer, buffer->data, buffer->pos); + +- /* First try to match any leftover at the start */ +- if (client->auth_end_offset > 0) ++ while (TRUE) + { +- gsize left = strlen (AUTH_END_STRING) - client->auth_end_offset; +- gsize to_match = MIN (left, buffer->pos); +- /* Matched at least up to to_match */ +- if (memcmp (buffer->data, &AUTH_END_STRING[client->auth_end_offset], to_match) == 0) ++ guint8 *line_start = client->auth_buffer->data + offset; ++ gsize remaining_data = client->auth_buffer->len - offset; ++ guint8 *line_end; ++ ++ line_end = memmem (line_start, remaining_data, ++ AUTH_LINE_SENTINEL, strlen (AUTH_LINE_SENTINEL)); ++ if (line_end) /* Found end of line */ + { +- client->auth_end_offset += to_match; ++ offset = (line_end + strlen (AUTH_LINE_SENTINEL) - line_start); + +- /* Matched all */ +- if (client->auth_end_offset == strlen (AUTH_END_STRING)) +- return to_match; ++ if (!auth_line_is_valid (line_start, line_end)) ++ return FIND_AUTH_END_ABORT; + +- /* Matched to end of buffer */ +- return -1; +- } ++ *line_end = 0; ++ if (auth_line_is_begin (line_start)) ++ return offset - original_size; + +- /* Did not actually match at start */ +- client->auth_end_offset = -1; +- } ++ /* continue with next line */ ++ } ++ else ++ { ++ /* No end-of-line in this buffer */ ++ g_byte_array_remove_range (client->auth_buffer, 0, offset); + +- /* Look for whole match inside buffer */ +- match = memmem (buffer, buffer->pos, +- AUTH_END_STRING, strlen (AUTH_END_STRING)); +- if (match != NULL) +- return match - buffer->data + strlen (AUTH_END_STRING); ++ /* Abort if more than 16k before newline, similar to what dbus-daemon does */ ++ if (client->auth_buffer->len >= 16*1024) ++ return FIND_AUTH_END_ABORT; + +- /* Record longest prefix match at the end */ +- for (i = MIN (strlen (AUTH_END_STRING) - 1, buffer->pos); i > 0; i--) +- { +- if (memcmp (buffer->data + buffer->pos - i, AUTH_END_STRING, i) == 0) +- { +- client->auth_end_offset = i; +- break; ++ return FIND_AUTH_END_CONTINUE; + } + } +- +- return -1; + } + + static gboolean +@@ -2306,6 +2349,14 @@ side_in_cb (GSocket *socket, GIOCondition condition, gpointer user_data) + if (extra_data > 0) + side->extra_input_data = g_bytes_new (buffer->data + buffer->size, extra_data); + } ++ else if (auth_end == FIND_AUTH_END_ABORT) ++ { ++ buffer_unref (buffer); ++ if (client->proxy->log_messages) ++ g_print ("Invalid AUTH line, aborting\n"); ++ side_closed (side); ++ break; ++ } + } + + got_buffer_from_side (side, buffer); diff --git a/SPECS/flatpak.spec b/SPECS/flatpak.spec index 40b639b..6d3b509 100644 --- a/SPECS/flatpak.spec +++ b/SPECS/flatpak.spec @@ -2,7 +2,7 @@ Name: flatpak Version: 0.8.8 -Release: 3%{?dist} +Release: 4%{?dist} Summary: Application deployment framework for desktop apps Group: Development/Tools @@ -14,6 +14,8 @@ Patch0: ostree-soup-Hold-a-ref-to-the-pending-URI-during-completion.patc Patch1: no-user-systemd.patch Patch2: fix-build.patch Patch3: ostree-bundle.patch +# https://bugzilla.redhat.com/show_bug.cgi?id=1547376 +Patch4: flatpak-0.8.8-cve-2018-6560.patch BuildRequires: pkgconfig(fuse) BuildRequires: pkgconfig(gio-unix-2.0) BuildRequires: pkgconfig(gobject-introspection-1.0) >= 1.40.0 @@ -117,6 +119,7 @@ cd .. %patch1 -p1 %patch2 -p1 %patch3 -p1 +%patch4 -p1 %build @@ -252,6 +255,9 @@ flatpak remote-list --system &> /dev/null || : %changelog +* Tue Aug 28 2018 David King - 0.8.8-4 +- Add patch for CVE-2018-6560 (#1547376) + * Mon Dec 11 2017 David King - 0.8.8-3 - Disable O_TMPFILE in libglnx (#1520311)