From 8d2f82226f97cb14dde023dd643fd3a2619f933c Mon Sep 17 00:00:00 2001 From: "M. Vefa Bicakci" Date: Thu, 26 Mar 2020 23:34:38 +0300 Subject: [PATCH] xside: Introduce override_redirect protection Prior to this commit, an application (malicious or not) could create a very large window with the override_redirect attribute set. If the window in question was large enough to prevent the user from interacting with the window manager and/or Qubes OS widgets, it was impossible to terminate the application and/or the hosting VM via regular means. Hence, this commit introduces a simple protection measure against very large windows that have the override_redirect attribute set. The protection works by unsetting the override_redirect attribute for windows that attempt to cover more than 90% of the screen. Doing so allows the user to move and/or minimize the windows in question. When the protection takes effect for the first time, the user is warned once with a persistent notification about what just happened and is informed of a way to disable this protection on a per-VM basis. ("persistent" notifications need to be clicked on to be dismissed.) The protection feature can be disabled via /etc/qubes/guid.conf in dom0, and this commit introduces an example in the aforementioned file along with an explanation to help users. --- gui-daemon/guid.conf | 19 +++++++++++ gui-daemon/xside.c | 79 ++++++++++++++++++++++++++++++++++++-------- gui-daemon/xside.h | 5 +++ 3 files changed, 90 insertions(+), 13 deletions(-) diff --git a/gui-daemon/guid.conf b/gui-daemon/guid.conf index f5ce5c83..941e7a60 100644 --- a/gui-daemon/guid.conf +++ b/gui-daemon/guid.conf @@ -4,6 +4,7 @@ global: { # default values #allow_fullscreen = false; + #override_redirect_protection = true; #allow_utf8_titles = false; # Change the lines below to change the secure copy and paste keyboard shortcuts # Before changing secure_(copy|paste)_sequence below because you need @@ -34,5 +35,23 @@ VM: { #example-media-vm: { # allow_fullscreen = true; #}; + # Example of a VM that may create very large windows that + # have the override_redirect flag set. This flag allows a + # window to unconditionally cover all other windows and + # causes the window manager to make it impossible to + # minimize or hide the window in question. + # + # Qubes OS will prevent a window having the override_redirect + # flag set from covering more than 90% of the screen as a + # protection measure. The protection measure unsets this + # flag and lets the window manager (and hence the user) + # control the window. + # + # If this causes issues with a VM's or an application's usage, + # please adapt this example to disable this protection for + # a specific VM. + #example-vm-with-large-unusual-windows: { + # override_redirect_protection = false; + #} }; diff --git a/gui-daemon/xside.c b/gui-daemon/xside.c index dd25bec0..21669dd5 100644 --- a/gui-daemon/xside.c +++ b/gui-daemon/xside.c @@ -100,7 +100,8 @@ static void release_mapped_mfns(Ghandles * g, struct windowdata *vm_window); static void print_backtrace(void); static void parse_cmdline_prop(Ghandles *g); -static void show_error_message (Ghandles * g, const char *msg) +static void show_message(Ghandles *g, const char *prefix, const char *msg, + gint timeout) { char message[1024]; NotifyNotification *notify; @@ -110,8 +111,9 @@ static void show_error_message (Ghandles * g, const char *msg) fprintf(stderr, "Failed to init notification subsystem\n"); return; } - snprintf(message, sizeof message, "ERROR(%s): %s", g->vmname, msg); + snprintf(message, sizeof message, "%s(%s): %s", prefix, g->vmname, msg); notify = notify_notification_new(message, NULL, g->cmdline_icon); + notify_notification_set_timeout(notify, timeout); if (!notify_notification_show(notify, NULL)) { fprintf(stderr, "Failed to send notification\n"); } @@ -122,6 +124,11 @@ static void show_error_message (Ghandles * g, const char *msg) notify_uninit(); } +static void show_error_message(Ghandles *g, const char *msg) +{ + show_message(g, "ERROR", msg, NOTIFY_EXPIRES_DEFAULT); +} + /* ask user when VM sent invalid message */ static int ask_whether_verify_failed(Ghandles * g, const char *cond) { @@ -1119,6 +1126,53 @@ static int force_on_screen(Ghandles * g, struct windowdata *vm_window, return do_move; } +static void set_override_redirect(Ghandles * g, struct windowdata *vm_window, + int req_override_redirect) +{ + static int warning_shown; + uint64_t avail, desired; + const char * warning_msg = "This VM has attempted to create a very large window " + "in a manner that would have prevented you from closing it and regaining " + "the control of Qubes OS\'s graphical user interface.\n\n" + "As a protection measure, the \"override_redirect\" flag of the window " + "in question has been unset. If this creates unexpected issues in the " + "handling of this VM\'s windows, please set \"override_redirect_protection\" " + "to \"false\" for this VM in /etc/qubes/guid.conf to disable this protection " + "measure and restart the VM.\n\n" + "This message will only appear once per VM per session. Please click on this " + "notification to close it."; + + req_override_redirect = !!req_override_redirect; + + avail = (uint64_t) g->root_width * (uint64_t) g->root_height; + desired = (uint64_t) vm_window->width * (uint64_t) vm_window->height; + + if (g->override_redirect_protection && req_override_redirect && + desired > ((avail * MAX_OVERRIDE_REDIRECT_PERCENTAGE) / 100)) { + req_override_redirect = 0; + + if (g->log_level > 0) + fprintf(stderr, + "%s unset override_redirect for " + "local 0x%x remote 0x%x, " + "window w=%u h=%u, root w=%d h=%d\n", + __func__, + (unsigned) vm_window->local_winid, + (unsigned) vm_window->remote_winid, + vm_window->width, vm_window->height, + g->root_width, g->root_height); + + /* Show a message to the user, but do this only once. */ + if (!warning_shown) { + show_message(g, "WARNING", warning_msg, + NOTIFY_EXPIRES_NEVER); + warning_shown = 1; + } + } + + vm_window->override_redirect = req_override_redirect; +} + /* handle local Xserver event: XConfigureEvent * after some checks/fixes send to relevant window in VM */ static void process_xevent_configure(Ghandles * g, const XConfigureEvent * ev) @@ -1185,7 +1239,7 @@ static void handle_configure_from_vm(Ghandles * g, struct windowdata *vm_window) { struct msg_configure untrusted_conf; int x, y; - unsigned width, height, override_redirect; + unsigned width, height; int conf_changed; read_struct(g->vchan, untrusted_conf); @@ -1207,10 +1261,6 @@ static void handle_configure_from_vm(Ghandles * g, struct windowdata *vm_window) width = untrusted_conf.width; height = untrusted_conf.height; VERIFY(width > 0 && height > 0); - if (untrusted_conf.override_redirect > 0) - override_redirect = 1; - else - override_redirect = 0; x = max(-MAX_WINDOW_WIDTH, min((int) untrusted_conf.x, MAX_WINDOW_WIDTH)); y = max(-MAX_WINDOW_HEIGHT, @@ -1221,7 +1271,7 @@ static void handle_configure_from_vm(Ghandles * g, struct windowdata *vm_window) conf_changed = 1; else conf_changed = 0; - vm_window->override_redirect = override_redirect; + set_override_redirect(g, vm_window, !!(untrusted_conf.override_redirect)); /* We do not allow a docked window to change its size, period. */ if (vm_window->is_docked) { @@ -1779,10 +1829,7 @@ static void handle_create(Ghandles * g, XID window) min((int) untrusted_crt.x, MAX_WINDOW_WIDTH)); vm_window->y = max(-MAX_WINDOW_HEIGHT, min((int) untrusted_crt.y, MAX_WINDOW_HEIGHT)); - if (untrusted_crt.override_redirect) - vm_window->override_redirect = 1; - else - vm_window->override_redirect = 0; + set_override_redirect(g, vm_window, !!(untrusted_crt.override_redirect)); parent = untrusted_crt.parent; /* sanitize end */ vm_window->remote_winid = window; @@ -2232,7 +2279,7 @@ static void handle_map(Ghandles * g, struct windowdata *vm_window) } else vm_window->transient_for = NULL; - vm_window->override_redirect = !!(untrusted_txt.override_redirect); + set_override_redirect(g, vm_window, !!(untrusted_txt.override_redirect)); attr.override_redirect = vm_window->override_redirect; XChangeWindowAttributes(g->display, vm_window->local_winid, CWOverrideRedirect, &attr); @@ -3178,6 +3225,7 @@ static void load_default_config_values(Ghandles * g) g->paste_seq_mask = ControlMask | ShiftMask; g->paste_seq_key = XK_v; g->allow_fullscreen = 0; + g->override_redirect_protection = 1; g->startup_timeout = 45; g->audio_low_latency = 1; g->trayicon_mode = TRAY_TINT; @@ -3262,6 +3310,11 @@ static void parse_vm_config(Ghandles * g, config_setting_t * group) g->allow_fullscreen = config_setting_get_bool(setting); } + if ((setting = + config_setting_get_member(group, "override_redirect_protection"))) { + g->override_redirect_protection = config_setting_get_bool(setting); + } + if ((setting = config_setting_get_member(group, "audio_low_latency"))) { g->audio_low_latency = config_setting_get_bool(setting); diff --git a/gui-daemon/xside.h b/gui-daemon/xside.h index 2f034686..314e8151 100644 --- a/gui-daemon/xside.h +++ b/gui-daemon/xside.h @@ -37,6 +37,9 @@ /* default width of forced colorful border */ #define BORDER_WIDTH 2 +/* maximum percentage of the screen that an override_redirect window can cover */ +#define MAX_OVERRIDE_REDIRECT_PERCENTAGE 90 + /* this makes any X11 error fatal (i.e. cause exit(1)). This behavior was the * case for a long time before introducing this option, so nothing really have * changed */ @@ -179,6 +182,8 @@ struct _global_handles { pid_t kill_on_connect; /* pid to kill when connection to gui agent is established */ int allow_utf8_titles; /* allow UTF-8 chars in window title */ int allow_fullscreen; /* allow fullscreen windows without decoration */ + int override_redirect_protection; /* disallow override_redirect windows to cover more than + MAX_OVERRIDE_REDIRECT_PERCENTAGE percent of the screen */ int copy_seq_mask; /* modifiers mask for secure-copy key sequence */ KeySym copy_seq_key; /* key for secure-copy key sequence */ int paste_seq_mask; /* modifiers mask for secure-paste key sequence */