On Mon, Aug 10, 2026 at 5:17 PM Peter Maydell <[email protected]> wrote:
> On Mon, 10 Aug 2026 at 15:07, Kostiantyn Kostiuk <[email protected]> > wrote: > > > > Before this commit, when qmp_guest_ssh_add_authorized_keys adds an > > SSH key for an existing local user, the agent (running as root) decides > > whether to create the user's .ssh directory with a symlink-following > > directory test, and then writes and chowns the authorized_keys file. > > A local unprivileged user who owns their home directory can pre-stage > > their .ssh directory (or the authorized_keys file) as a symbolic link > > so that, when the host or operator triggers a key add for that user, > > the root agent follows the link and transfers ownership of an arbitrary > > root-owned file or directory to the unprivileged user, who can then > rewrite > > it to obtain root. > > > > Fixes: CVE-2026-12080 > > Fixes: https://gitlab.com/qemu-project/qemu/-/work_items/3929 > > > > v1: https://patchew.org/QEMU/[email protected]/ > > v2 -> v1: > > Change effective user/group ID instead of checking for symlinks and > > changing ownership of the file. > > > > v2: https://patchew.org/QEMU/[email protected] > > v3 -> v2: > > Deduplicate code. > > Fail daemon when can't rollback effective user/group ID. > > > > Reported-by: Valentino Paulon <[email protected]> > > Signed-off-by: Kostiantyn Kostiuk <[email protected]> > > --- > > qga/commands-posix-ssh.c | 66 ++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 66 insertions(+) > > > > diff --git a/qga/commands-posix-ssh.c b/qga/commands-posix-ssh.c > > index 661972e34e..070cdffcd0 100644 > > --- a/qga/commands-posix-ssh.c > > +++ b/qga/commands-posix-ssh.c > > @@ -8,11 +8,29 @@ > > #include <glib/gstdio.h> > > #include <locale.h> > > #include <pwd.h> > > +#include <grp.h> > > > > #include "commands-common-ssh.h" > > #include "qapi/error.h" > > #include "qga-qapi-commands.h" > > > > +typedef struct EffectiveUserInfo { > > + uid_t uid; > > + gid_t gid; > > +} EffectiveUserInfo; > > + > > +typedef EffectiveUserInfo *PEffectiveUserInfo; > > + > > +static void rollback_effective_info(PEffectiveUserInfo info) > > +{ > > + if (info) { > > + assert(seteuid(info->uid) == 0); > > + assert(setegid(info->gid) == 0); > > A brief comment here about why we're happy to assert() here would > be helpful. > Done > > > + } > > +} > > + > > +G_DEFINE_AUTO_CLEANUP_FREE_FUNC(PEffectiveUserInfo, > rollback_effective_info, NULL); > > Don't we also need to g_free(info) in the cleanup function? > You are right > > thanks > -- PMM > >
