Skip to content

Commit

Permalink
xside: Introduce override_redirect protection
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@m-v-b
m-v-b committed Mar 30, 2020
1 parent da71c29 commit 8d2f822
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 13 deletions.
19 changes: 19 additions & 0 deletions gui-daemon/guid.conf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
#}
};

79 changes: 66 additions & 13 deletions gui-daemon/xside.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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");
}
Expand All @@ -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)
{
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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);
Expand All @@ -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,
Expand All @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
5 changes: 5 additions & 0 deletions gui-daemon/xside.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -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 */
Expand Down

0 comments on commit 8d2f822

Please sign in to comment.