-
Notifications
You must be signed in to change notification settings - Fork 578
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
Security fix breaks guake -e
#2042
Comments
Not sure how D-Bus work, but can we somehow limit D-Bus access only to the command from the same user running guake, and restore the functionality? From what I read, D-Bus is kind of an IPC mechanic, very much like HTTP. I know so little about this thing, but if nothing else works, can we just create a secret (e.g. from: /dev/urandom) known only to guake, and use HMAC + timestamp + some other things to sign the command send via D-Bus. This way, only guake, can communicate with its own instances; denying access from some random application. |
Hrm, when Guake is run via terminal it's always going to be a separate instance from the one you want to interact with. It contacts the current running Guake instance via dbus by name, and then it can send commands. Somehow, we need to generate a secret that only a running instance of Guake and another instance of Guake can access, a random secret of some kind might be nice if there's a way to only make it viewable to instances of Guake. Not sure how we can pass the secret between the two instances in a way that can't be replicated by other programs. Actually, the exploit that I was alerted to and dealt with would allow malicious actors to gain the ability to execute commands as root if Guake is in a super user state. Since the the goal of the exploit is to gain root, if there's a secret stored in a location only accessible with super user privileges, I think that deals with the issue. Most people aren't running Guake with sudo normally, so it wouldn't normally have the ability to access information in a place with root privileges so I think there'll need to be a one-time setup of a public/private key pair. So the way it'd go is:
Have I missed an angle of attack here? I can run this by the person who alerted us to the initial issue as well, since they were savvy enough to spot the original vulnerability. |
If we are going that route, why not generate the secret on the first run? For instance, store it as a flat file at |
Hum, is that sufficient? I'll include that in the message I send the security guy. Wouldn't this allow another installed program running at user level to perform the exploit? |
If they have access to the file with 0600 permission, they have no reason to try running a command as that user; they already have higher privileges. I'm glad we can discuss solutions and hope we can come up with a good resolution. |
I'm not a D-Bus expert, but if I remember correctly, there are two type of D-Buses - one "system D-Bus" and more "session D-Buses", which are only accessible in a given individual user login sessions. Excerpt from Wikipedia: https://en.wikipedia.org/wiki/D-Bus |
Still waiting on the reply from the security guy and was a bit busy last few days, can take a shot at this while waiting maybe. Going to try get it checked more than usual, really don't want to accidentally reintroduce the issue. |
Initial implementation with notes in #2051. File set with 400 permissions post creation, exploit that was initially reported still possible, may need revision. |
Any news? By the way, how does other terminals like |
Sorry, I've studied the proposed solution a bit and I feel that it doesn't solve the problem. After all, the basic problem is that the background guake process that handles D-BUS calls is (stupidly) running as root. Isn't it easier to just add to original dbusiface.py something like this:
(sorry - I no speak Pythonicano 😥) Because if I'm regular (non-root) user and my guake is running under my effective id, there is IMHO no security limitation to use execute_command via DBUS call and there is no need to mess with "sudo" and "cryptography". |
Actually, Guake can execute root commands even when it isn't running as root. As a terminal, a Guake session that isn't running as root can be escalated to root privileges by typing sudo, which you likely do often, or by entering Although, actually, maybe there's something in the VTE docs for checking if the terminal is in a root state. Maybe this is better than requiring that the |
@ivoshm |
That's not true (at least) on my Manjaro - Guake uses session D-BUS which is connected to my user session in graphical environment. Other users (except "root" of course) can't connect to it. Example with Guake 3.8.1 when user1001 try to send command to user1000 session D-BUS
|
I got it - attack vector is possibility to send commands into terminal with "elevated" shell. Quick and dirty solution:
Just for record: terminal including sudo cached credentials is identifiable by exit code of |
Checking ID catches if the user in If the -e command was being used solely for startup scripts, what about reforming it to always run the command in a new tab? I think that dodges any escalated privilege hijacking concerns, just need a moment to try and brainstorm any way a fresh shell could still have root. |
Let me ping @ghostshadow to see if he has other inputs. He seems to be another person who cares about this feature. To summarize, based on my understanding, the main topic is not about D-Bus anymore; letting other terminals input something into guake is the issue in itself. Even with the same user, there can be risks of privilege escalation by other means (e.g. sudo memorization).
Do you guys actually want to control guake from other terminals or just want to configure startup tabs? |
Sorry for the late response, I'm busy lately.
Yes, I do use this feature as to configure my startup tabs, not to control Here is the code I'm using: So, it makes sense to have this feature to affect only new tabs, which I believe what the General Options:
-h, --help; -V, --version; --disable-server; --color-table; --preferences;
--default-display=display; --default-working-directory=directory
...
Tab Options:
-x, --execute; -e, --command=command; -T, --title=title;
--dynamic-title-mode=mode ('replace', 'before', 'after', 'none');
--initial-title=title; --working-directory=directory; -H, --hold;
--active-tab; --color-text=color; --color-bg=color
Window Options:
--display=display; --geometry=geometry; --role=role; --drop-down;
--startup-id=string; -I, --icon=icon; --fullscreen; --maximize; --minimize;
--show-menubar, --hide-menubar; --show-borders, --hide-borders;
--show-toolbar, --hide-toolbar; --show-scrollbar, --hide-scrollbar;
--font=font; --zoom=zoom makes me think that executing commands from other terminals is not allowed but for spawning a new tab. You cannot spawn a new window with an arbitrary command neither do tell a running instance to execute a command remotely. |
I couldn't think of any way to hijack privileges on a fresh terminal so I'm open to going that way too. I'll put it to a vote, all in favor of reforming -e to only make a fresh tab to run in leave a spaceship reaction, all in favor of making -e sudo leave an eyes emoji |
I was thinking something along the line of dedicated config sections for startup tabs; it can be something simple, like JSON config. If there needs to be something between these two, the rocket one works for me too. |
New approach created, was significantly simpler, still want to see it get reviewed, @mlouielu, just to make sure I haven't gone and reintroduced a hole somewhere |
Oh, I am relieved that this is finally settled. It has been a nice discussion. Good work everyone, cya. |
Any updates on this issue? Since its still not working :-( |
Merged the resolution after far too much waiting. I'm just going to stop requesting reviews since other reviewers are fully absent now |
Can we bring back the original functionality somehow? Even if that means that you have to use a token or an authentication, whatever I don't know, but I think we cut down too much for security here. I think security is paramount, but the solution is not to remove features but to make them secure. I used to use execute_command to run specific commands on my guake tabs from vim. Can we just allow commands that target non-root tabs. I think that security would be the same as the existing new-tab functionality. |
@vasilakisfil The vulnerability initially focused on dBus; however, later on, it was determined that letting 3rd party's application pose as if they are users themself is a problem; these include exposing an interface with the same effect as a user typing into the terminal. Even with non-root tabs, the |
Still having the issue, did I missed something? ^^
$ guake --supportGuake Version: 3.9.0 Vte Version: 0.68.0 Vte Runtime Version: 0.68.0 GTK+ Version: 3.24.33 GDK Backend: <GdkX11.X11Display Desktop Session: ubuntu-xorg Display: :0 |
3.9 should be it, is there an additional older version of guake installed running that's receiving the commands? |
Does this issue mean you cannot start guake from another command in a specific directory? |
This seems to be the case and breaks integration with Open Terminal Here... and Run in Terminal... file manager features. I came up with a workaround : #!/bin/sh
if [ $# -lt 2 ]; then
guake --show --new-tab "$PWD";
exit 0;
fi
while getopts "e:" flag; do
case $flag in
e) dir="$PWD";
cmd="$OPTARG";
[ -f "$OPTARG" ] &&
dir="$(dirname $(realpath $OPTARG))" &&
cmd="./$(basename $OPTARG)";
guake --show -e "cd $dir && clear && $cmd";
break
;;
\?) exit 1
;;
esac
done Call this script with no arguments to open Guake in a new tab at the inherited working dir. Call it with the For example : $ cd ~/test
$ guake-wrapper
// $PWD == /home/you/test
$ guake-wrapper -e ls
// $PWD == /home/you/test
$ guake-wrapper -e "bash -c 'echo \"testing 123\"'"
// $PWD == /home/you/test
$ guake-wrapper -e ./project/test.sh
// $PWD == /home/you/test/project
$ guake-wrapper -e /path/to/script.sh
// $PWD == /path/to/script FYI, to get this working in Nemo File Manager, you have to symlink |
In version 3.8.5,
execute_command
has been removed from DBus#1796
The discussion there implies that
-e
option won't affect by this change.In reality, it does break the functionality
Here the stack trace
To reproduce, simply try
-e
option.$guake --support
The text was updated successfully, but these errors were encountered: