Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve protection against application windows with the "override redirect" flag #4705

Open
SuzanneSoy opened this issue Jan 11, 2019 · 26 comments
Labels
C: gui-virtualization fixed-by-wayland Bugs that would not appear if Qubes OS used Wayland instead of X11 P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. security This issue pertains to the security of Qubes OS. T: enhancement Type: enhancement. A new feature that does not yet exist or improvement of existing functionality.

Comments

@SuzanneSoy
Copy link

Qubes OS version:

R4.0

Affected component(s):

Tested on an up-to-date debian-9 AppVM.


Steps to reproduce the behavior:

sudo apt -y install xfce4-session vnc4server tigervnc-viewer
vncserver :1
# type a dummy password

Expected behavior:

No window should be able to go fullscreen without user interaction (/etc/qubes/guid.conf does not contain allow_fullscreen = true). For example, pressing F11 in Firefox (which is a Firefox shortcut, not a window manager one) maximises the window, but does not let it actually go fullscreen.

Actual behavior:

A fullscreen window opens. There is a 1px border visible that hints that something is wrong, but otherwise it can draw over the entire desktop, including the Dom0 menu at the top of the screen. It also manages to steal some keyboard shortcuts, including Ctrl+Shift+P, and partially steals Alt+Tab (the focus indeed goes from one window to another, but it seems that windows that belong to another VM cannot be brought forwards. It also manages to prevent input to other windows most of the time. Sometimes mouse input is possible in other windows (as can be observed if inactive windows are set to transparent in Dom0's compositor options), but after a few Alt+Tab it is impossible to click anywhere (the mouse cursor moves, but keyboard input and clicks do not work anymore).

General notes:

A malicious VM could exploit the same X11 calls to produce a somewhat credible fake desktop (which couldn't access other VM's data of course, but could still be dangerous for password input etc.).


Related issues:

@marmarek
Copy link
Member

The 2px border is what is enforced to make user notice it's VM window.
A little background - this is all about windows with "override redirect" flag set. It basically request window manager to not do anything about such window (including adjusting its position, adding decoration etc). This is quite obscure X protocol feature... It is mostly used for menus, drop downs and generally parts of other windows that for some reason needs to be a separate window in X protocol sense. But there are also applications that abuse it for other things. You just found one of such application (there is proper way for application to request "fullscreen window" from a window manager, but apparently this one doesn't like it).

Ideally we wouldn't allow such windows (evading window manager) at all, but unfortunately this breaks a lot of things, basically makes the system unusable. Think of problems like menus opening in a totally different place on the screen, or even below the current window, or closing immediately as application notice it doesn't have focus anymore.
Some years ago I've tried emulating "override redirect" windows with window-manager managed windows, but every window options combination I could think of, leads to some applications broken.

Some workaround would be to limit maximum size of such windows. Like half of the screen at most. This isn't bulletproof (one could use two or more windows to cover the screen), but should be easier to notice. But also this may lead to breaking some applications (for example maximized Firefox can have quite a big drop down menus for history or open tabs).

I think the most problematic thing is those windows generally stays on top, even if you switch focus with alt+tab. I haven't found any way to avoid this (while preserving other "override redirect" properties), but maybe I'm missing something?

Related: #881

@andrewdavidwong andrewdavidwong changed the title tigervnc goes fullscreen without user interaction, can spoof most of the desktop Improve protection against application windows with the "override redirect" flag Jan 14, 2019
@andrewdavidwong andrewdavidwong added T: enhancement Type: enhancement. A new feature that does not yet exist or improvement of existing functionality. C: gui-virtualization security This issue pertains to the security of Qubes OS. labels Jan 14, 2019
@andrewdavidwong andrewdavidwong added this to the Far in the future milestone Jan 14, 2019
@SuzanneSoy
Copy link
Author

Ah, I had read #881 but didn't realize this was an instance of an override redirect window.

Some random ideas (not enough to actually fix the problem, so not worth wasting time implementing them to get a half-broken result that can still be circumvented).

  • Lie about the screen resolution and prevent override_redirect windows larger than screen_height-40px (roughly, so that these windows may not cover the task bar and the title bar of a maximized window).
    • That way, even if an override redirect window tries to cover as much screen as it can, it will be clearly visible.
    • If an override redirect window tries to cover as little as possible, to steal focus, then we have a problem.
  • Make sure these windows cannot steal ctrl+shift+p and other privileged shortcuts.

@DemiMarie
Copy link

Could Qubes ensure that an override_redirect window is entirely contained in a normal window owned by the same AppVM?

@marmarek
Copy link
Member

That would break many applications, as in many cases the whole reason for using separate window is that it can extend outside of its parent. See for example network manager menu, or basically any menu with sufficiently small application window.

@DemiMarie
Copy link

DemiMarie commented Jan 16, 2019 via email

@marmarek
Copy link
Member

I recommend you to look for such "override redirect" windows on your system, they are easy to distinguish - have 2px border and no title bar. You'll see most of the legitimate use cases for them violate at least some of the points above... Some examples:

  • Must have a full border, including title

This breaks (all?) combobox widgets, as such title bar would either overlap with the thing you're selecting, or shift the whole thing down (which in many cases make the application close it for some reason).

  • Must be docked to the status bar

Try right click anywhere.

  • Cannot overlap any windows belonging to a different VM or to dom0

Again, right click, near window edge. Or widget menu.

  • Cannot overlap the dom0 status bar

All widget menus

  • Cannot receive input for at least five seconds after spawning

Waiting 5sec before clicking anything in any menu is not a good idea, at least...

  • The total area of all such windows (from all VMs) cannot exceed 20% of the screen.

This actually may be a good idea. If the criteria is not met, setting "override redirect" flag is rejected and window get normal borders, title bar and is managed by window manager. In many cases such forcing no-override-redirect will break application badly, but maybe those few that abuse it for just "large window" will still work?
Enforcing it may be tricky, possible problems I see now:

  • various race conditions when checking area covered by other VMs
  • windows can be resized, and I'm not entirely sure if it's possible to drop override-redirect flag when window is already visible

Obviously, windows that are entirely within existing windows owned by the same VM are exempt.

This is trivial to bypass - simply create normal window (or windows) below, open new override-redirect window covering them, then close those below (or not). Note also that above/below relation can change - while such window can be perfectly legal at one time, as soon you press alt+tab it's not legal anymore.

I wonder how Wayland compositors handle such windows (menus, popups etc).

@DemiMarie
Copy link

DemiMarie commented Jan 16, 2019 via email

@marmarek
Copy link
Member

I believe those applications are broken on Wayland.

They are definitely not. We are talking here about anything that window can open. This include any menu etc. Try a simple thing: right click on this very window you're reading this message in. What you see now is a "override redirect" window, which have position, size etc controlled by that very application (to appear near mouse cursor or other parent-window matching place).
I've just tried the same thing on (baremetal) Fedora 28 running Wayland. Here is the result:
screenshot

As you can see, the menu extends below the main window. I'm not sure if that's "separate window" in Wayland terminology, but definitely it is handled somehow.

Combo boxes can (and should) be handled by dom0.

I hope you're not suggesting moving the whole toolkit (working on individual widgets level) to dom0. That would have enormous attack surface.

@DemiMarie
Copy link

DemiMarie commented Jan 16, 2019

I believe those applications are broken on Wayland.

They are definitely not. We are talking here about anything that window can open. This include any menu etc. Try a simple thing: right click on this very window you're reading this message in. What you see now is a "override redirect" window, which have position, size etc controlled by that very application (to appear near mouse cursor or other parent-window matching place).
I've just tried the same thing on (baremetal) Fedora 28 running Wayland. Here is the result:
screenshot

As you can see, the menu extends below the main window. I'm not sure if that's "separate window" in Wayland terminology, but definitely it is handled somehow.

Combo boxes can (and should) be handled by dom0.

I hope you're not suggesting moving the whole toolkit (working on individual widgets level) to dom0. That would have enormous attack surface.

facepalm I was thinking of the widgets that are in the status bar at the top right (qui-domains et al), most of which are already in dom0. I am not sure what the correct term for those is, however. @marmarek, I apologise for the confusion.

I was obviously confused by what an override redirect window was. That said, I do believe:

  • No domU-controlled window should ever overlap the status bar at the top of the screen.
  • No domU-created override-redirect window should be able to steal focus from a window that is controlled by another VM or by dom0.
  • One could kill an override-redirect window that is contained by a normal window that itself closes (whether it is a good idea is another question).
  • It might be a good idea to forbid a normal window from stealing focus from a window that belongs to another domain.

@tasket
Copy link

tasket commented Feb 6, 2019

I can think of other creative ways to deal with this:

A) Temporarily flashing a large border + VM label on the perimeter which could last 5-10 sec. Alternately, the border+label could remain visible until the user offers some input, such as a mouse click. This probably requires using some dom0 compositing feature (even if 2D) or some other way to keep the underlying window buffer intact.

B) Activating a tooltip for the window in an especially conspicuous way. Or a temporary floating label that is similar to a tooltip.

C) Don't force the resolution, but do force positioning of the window. This allows a normal border to be present although part of the window will hang off the edge of the screen.

@andrewdavidwong andrewdavidwong added the P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. label Mar 27, 2019
@kajmagnus
Copy link

kajmagnus commented Oct 23, 2019

( In case anyone uses IntelliJ IDEA (an IDE written in Java): I also had this problem with Qubes 4 + IntelliJ IDEA, and, recently I upgraded to the newest IDEA version, and now, the problem is gone :- ) There're borders around the popup windows in IDEA (those windows which previously didn't have any borders and disappeared in a blink) — I'm wondering if Jetbrains (who develop IDEA) did some change, that solved this issue ... for IntelliJ IDEA only I'd guess; I'd guess other Java apps still have this problem. )

marmarek pushed a commit to marmarek/qubes-gui-daemon that referenced this issue May 17, 2020
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.

(cherry picked from commit 8d2f822)

QubesOS/qubes-issues#4705
@DemiMarie
Copy link

@marmarek A few extra suggestions:

  • Block all non-dom0 windows (including ones with override_redirect set) from covering the taskbar at the top of the screen. Firefox does this, and it is quite annoying.
  • Draw a full border around override redirect windows, either by default or as an option.
  • Require override redirect windows to overlap a standard window owned by the VM, or else be within a certain portion of the screen.

@marmarek
Copy link
Member

marmarek commented Jun 1, 2020

* Firefox does this, and it is quite annoying.

Which version? Firefox 75 doesn't do that for me.

Draw a full border around override redirect windows, either by default or as an option.

This is a bad idea, see #4705 (comment)
But @m-v-b implemented a lighter version in QubesOS/qubes-gui-daemon#41 - prevent override redirect flag if the window is too big (more than 90% of the screen).

* Require override redirect windows to overlap a standard window owned by the VM, or else be within a certain portion of the screen.

This is another heuristic that could indeed work, but IMO has much more corner cases (around definition of "overlap").

@DemiMarie
Copy link

* Firefox does this, and it is quite annoying.

Which version? Firefox 75 doesn't do that for me.

76 if I recall correctly. It only does this if microphone and/or camera are being shared.

Draw a full border around override redirect windows, either by default or as an option.

This is a bad idea, see #4705 (comment)

What if we just drew the border, without changing anything else?

* Require override redirect windows to overlap a standard window owned by the VM, or else be within a certain portion of the screen.

This is another heuristic that could indeed work, but IMO has much more corner cases (around definition of "overlap").

What about “override redirect windows cannot steal focus from a different VM”?

@DemiMarie
Copy link

In the long run, we might want to switch to Wayland.

@marmarek
Copy link
Member

marmarek commented Jun 2, 2020

76 if I recall correctly. It only does this if microphone and/or camera are being shared.

Ah, so you mean that small rectangle with mic icon? I think that's actually a feature, to have it clearly visible, always, that some website is listening.

What if we just drew the border, without changing anything else?

Technically it would be tricky (normally window manager draws those, but override redirect windows are excluded form window manager handling), but also it wouldn't help with the main issue - that those windows are not included in normal focus switching, cannot be moved etc - exactly because window manager doesn't handle them. Please read the comment I linked for details.

What about “override redirect windows cannot steal focus from a different VM”?

Again - focus handling is weird in relation to those windows. IIUC it isn't possible to switch focus to such window (window manager won't let you do that). Preventing gaining a focus may prevent some malicious usage of such windows, but also will break many applications (at least network manager applet, which formally doesn't have focus).

In the long run, we might want to switch to Wayland.

Yes, it may eliminate this problem. And introduce a new set of problems ;)

Actually, it would be interesting to see how override-redirect windows are handled by xwayland.

@andrewdavidwong
Copy link
Member

The warning message might need some improvement to be understandable by end users:

https://qubes-os.discourse.group/t/warning-message-from-qubes-rpc/500

@m-v-b
Copy link

m-v-b commented Sep 10, 2020

After reading the user report linked by @andrewdavidwong , I realized that the warning message is too technical, and potentially not helpful. Furthermore, as mentioned by @juodumas in this comment, it looks like LibreOffice on Debian 10-based AppVMs triggers the same warning. The fact that this warning is triggered by a commonly used application may cause users to learn to ignore this warning (after all, what can be done, aside from disabling the override redirect protection for the affected AppVMs?), reducing the warning's value.

Perhaps we can remove the override-redirect protection-related warning message in question from the code altogether? If there is consensus on the removal of the warning message, I can prepare a pull request.

@andrewdavidwong
Copy link
Member

If I understand correctly:

  • If we don't have a warning message, the risk is that a user legitimately needs to disable the protection but doesn't know how (or what happened).
  • If we do have a warning message, the risk is that it triggers too often, user's don't understand what it means, and they learn to ignore it.

Maybe @ninavizz has an opinion?

@marmarek
Copy link
Member

marmarek commented Nov 3, 2020

* If we **do** have a warning message, the risk is that it triggers too often, user's don't understand what it means, and they learn to ignore it.

Or even worse - disable the protection based on instructions there to get rid of the notification, even if nothing is really broken.

I think it's better to remove the warning notification (but still write it into the log).

@ninavizz
Copy link
Member

ninavizz commented Nov 5, 2020

I definitely feel that warning is confusing. The pattern that's being described above is also very confusing, and unique to Qubes OS in a fashion that feels necessary; but could use some refinement. Will keep on my plate to look at in the future when we have the bandwidth to reconsider broader desktop experience things.

@DemiMarie
Copy link

76 if I recall correctly. It only does this if microphone and/or camera are being shared.

Ah, so you mean that small rectangle with mic icon? I think that's actually a feature, to have it clearly visible, always, that some website is listening.

It probably is, but I think it is reasonable to impose limits on such windows. For instance, they could be converted to normal titlebar windows, and limited in number per VM. I would also be happy to just ban them from the task bar; in the case of QubesOS, it is are more annoying than it is beneficial.

@marmarek
Copy link
Member

I would very much welcome a patch that do any of the above, and do not break legitimate applications.

@DemiMarie
Copy link

My current plan:

  • Add several options to qubes-guid:
    • --override-redirect=disabled: disable override redirect entirely
    • --override-redirect=restricted: override redirect windows cannot cover dom0 toolbar and must overlap a normal window; this will hopefully become the default.
    • --override-redirect=transform: transform override redirect windows into normal windows, and try to emulate their behavior in other ways.
    • --override-redirect=allow: current behavior
  • After R4.1, switch to Wayland for R4.2.

@ninavizz
Copy link
Member

@DemiMarie Might you be open to do a synchronous screenshare/call this week to discuss? Honestly curious what this issue, #6347, and #6931 all speak to and what remediations may imply for users.

@DemiMarie
Copy link

@DemiMarie Might you be open to do a synchronous screenshare/call this week to discuss? Honestly curious what this issue, #6347, and #6931 all speak to and what remediations may imply for users.

Absolutely!

@andrewdavidwong andrewdavidwong removed this from the Release TBD milestone Aug 13, 2023
@DemiMarie DemiMarie added the fixed-by-wayland Bugs that would not appear if Qubes OS used Wayland instead of X11 label Aug 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: gui-virtualization fixed-by-wayland Bugs that would not appear if Qubes OS used Wayland instead of X11 P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. security This issue pertains to the security of Qubes OS. T: enhancement Type: enhancement. A new feature that does not yet exist or improvement of existing functionality.
Projects
None yet
Development

No branches or pull requests

8 participants