Tudmotu / gnome-shell-extension-clipboard-indicator

The most popular clipboard manager for GNOME, with over 1M downloads
https://extensions.gnome.org/extension/779/clipboard-indicator/
MIT License
818 stars 166 forks source link

RCE vulnerability when copying full system paths #453

Closed Edu4rdSHL closed 2 months ago

Edu4rdSHL commented 3 months ago

The entry parsing is broken, and I don't even know whether the underlying issue is the extension or something the extension depends on. It leads to a RCE, basically. If you copy a full system path, e.g /usr/bin/ls and the mimetype gets set to UTF8_STRING it will get executed/opened on the system! And the contents: key is replaced by that. For the example mentioned before about /usr/bin/ls, it's what you get:

https://gist.githubusercontent.com/Edu4rdSHL/20d05e03dfe4777eca76dba3c68d0f63/raw/587b6f38a62329e0d09d9bd992616195835a5b62/gistfile1.txt

It does look bad, and I'm not familiar with developing extensions to see where and why is this happening.

Regards, Ed

Edu4rdSHL commented 3 months ago

As I said, I'm not an js expert or even competent and it may be nonsense, but can that be caused by this line?

Tudmotu commented 2 months ago

Thank you @Edu4rdSHL for reporting this.

@andyholmes would you mind explaining why does this happen (I assume you understand because you fixed it)? I cannot understand how would the extension execute a command 😳

andyholmes commented 2 months ago

To be honest, I didn't trace it all the way down. The only thing that makes sense is that GdkPixbuf takes the bytes it's given and somehow ends up executing it.

I guess maybe some image formats, like maybe animated GIFs, are executables in some sense so it's capable of doing that? It sure sounds like something ridiculous an image processing library would do :/

Edu4rdSHL commented 2 months ago

The thing is that it's opening binary files too, such as /usr/bin/ls, and it's happening when a specific mimetype is found. I really have no idea why would even a clipboard library would try to load file contents.

Edu4rdSHL commented 2 months ago

I would even be that optimistic that if the affected mimetype is removed from the list, the issue wouldn't happen anymore, but that just a naive assumption.

andyholmes commented 2 months ago

I really have no idea why would even a clipboard library would try to load file contents.

So that it can do things like this:

Screenshot of Clipboard Indicator displaying a thumbnail of an image in the clipboard

There's no way to display a image without the contents of the image.

Edu4rdSHL commented 2 months ago

Yeah, I understand it for images, but why for a UTF-8 string.

andyholmes commented 2 months ago

I can't answer why the extension loads the contents of text files, rather than just displaying the filename.

Either way the bug would still exist, since I doubt there's a way for the clipboard implementation or anyone else to determine whether the mime-type is actually accurate. The source application just says "here's a blob of data, and here's the mime-type you should advertise it as".

In this case, it's just fortunate we can rely on Gio checking the content type (somewhat slowly) and not have to trust the mime-type.

Tudmotu commented 2 months ago

Oh damn, ok. It would be weird for Pixbuf to execute the binary data but that sounds like a possible explanation and I have no alternative explanation.

@Edu4rdSHL I think it's like this:

The [incomplete] theory goes like this: sometimes, the extensions tries to create an St.Icon from a file that is not an image. For this to happen, two things need to coincide:

  1. You copy a string of a path
  2. The extension identifies the entry as an image

If these two things happen, the extension will load the binary data of the path you copied and try to create an St.Icon from it.

What I'm still missing here is: Why does the extension identify the UTF8_STRING as an image that should be loaded into a thumbnail? I can't see where the bug is.

But I understand how @andyholmes' solution solves this.

Edu4rdSHL commented 2 months ago

Thank you so much for the detailed explanation @Tudmotu, and thanks @andyholmes for the fix!

Will be there a release including this fix on the extensions website soon?

Tudmotu commented 2 months ago

@Edu4rdSHL Yes, I uploaded it to e.g.o now: https://extensions.gnome.org/review/53581

It might take a bit before it's approved.

Edu4rdSHL commented 2 months ago

Thanks! I will try to get it approved asap with the extensions team.

andyholmes commented 2 months ago

Sometimes the mime type is "UTF8_STRING". Not sure why. Anyway I had to add support for it because some applications advertise their content as "UTF8_STRING" even though it's not an actual mime type

Ah, I found this in St.Clipboard some time ago:

const char *supported_mimetypes[] = {
  "text/plain;charset=utf-8",
  "UTF8_STRING",
  "text/plain",
  "STRING",
};

I would not count on that being a complete list though, because honestly why are there four to begin with, and charset= sure looks like it could hold other values :/

Tudmotu commented 2 months ago

Yeah, currently the extension tries to mimic St.Clipboard by choosing the mimetype based on a hard-coded list and querying the clipboard for these mimetypes until it finds one that has a value.

I hope this will be solved once I get to implementing the multi-mimetype support in Mutter. Unfortunately I'm not sure when I'll get to that.

Once Mutter has multi-mimetype support, this extension will no longer be opinionated on which mimetype it should keep in the history file ― it would just keep all of them. Right now it kind of "guesses" which mimetype is the most appropriate (based on the first mimetype that returns a value from the clipboard).

andyholmes commented 2 months ago

Ah gotcha, that would be good. I didn't realize Mutter was missing that, since I believe the portal-side advertises API support.

Tudmotu commented 2 months ago

Yeah technically the interface supports multiple mimetypes, both Wayland and X11 have multi-mimetype implementations but SelectionSourceMemory in Mutter doesn't. And since the Wayland/X11 implementations are not exposed via Gjs, this extension cannot utilize them, and we must add support in the Mutter implementation, which is exposed.