diff mbox series

[1/1] linux-user: Handle /proc/self/exe in syscall execve

Message ID 20190807135458.32440-2-dion@linutronix.de (mailing list archive)
State New, archived
Headers show
Series Handle /proc/self/exe in execve | expand

Commit Message

dion@linutronix.de Aug. 7, 2019, 1:54 p.m. UTC
From: Olivier Dion <dion@linutronix.de>

If not handled, QEMU will execve itself instead of the emulated
process.  This could result in potential security risk.

Signed-off-by: Olivier Dion <dion@linutronix.de>
---
 linux-user/syscall.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

Comments

Laurent Vivier Aug. 23, 2019, 4:58 p.m. UTC | #1
Le 07/08/2019 à 15:54, dion@linutronix.de a écrit :
> From: Olivier Dion <dion@linutronix.de>
> 
> If not handled, QEMU will execve itself instead of the emulated
> process.  This could result in potential security risk.
> 

Could you explain what you mean by potential security risk?

Thanks,
Laurent
Olivier Dion Sept. 2, 2019, 5:36 p.m. UTC | #2
On 2019-08-23T12:58:43-0400, Laurent Vivier <laurent@vivier.eu> wrote:

> Le 07/08/2019 à 15:54, dion@linutronix.de a écrit :
> > From: Olivier Dion <dion@linutronix.de>
> >
> > If not handled, QEMU will execve itself instead of the emulated
> > process.  This could result in potential security risk.
> >

> Could you explain what you mean by potential security risk?

I don't have any exploit in mind, but someone motivated enough could
certainly find one.  For example, it's possible to ask qemu static to
execute another program.

The main point is that an emulator should never leak informations to its
environnement.  If the emulated program can determine that it is being
emulated, other than by an "official" way, then the emulator is at
fault.
Laurent Vivier Sept. 2, 2019, 7:02 p.m. UTC | #3
Le 02/09/2019 à 19:36, Olivier Dion a écrit :
> 
> On 2019-08-23T12:58:43-0400, Laurent Vivier <laurent@vivier.eu> wrote:
> 
>> Le 07/08/2019 à 15:54, dion@linutronix.de a écrit :
>>> From: Olivier Dion <dion@linutronix.de>
>>>
>>> If not handled, QEMU will execve itself instead of the emulated
>>> process.  This could result in potential security risk.
>>>
> 
>> Could you explain what you mean by potential security risk?
> 
> I don't have any exploit in mind, but someone motivated enough could
> certainly find one.  For example, it's possible to ask qemu static to
> execute another program.

In the actual state, executing /proc/self/exe executes QEMU instead of
the binary and this is a minor bug not a security risk.

> The main point is that an emulator should never leak informations to its
> environnement.  If the emulated program can determine that it is being
> emulated, other than by an "official" way, then the emulator is at
> fault.

It should never leak _crucial_ information (like the serial number of
the host), but all emulators/hypervisors leak information (try to run
lscpu/lspci in a VM). In this case, again, I don't see any security risk.

Moreover qemu-user doesn't have kernel part and it has no way to elevate
privilege by itself (BTW you must not run it with suid bit).

We don't have a nice solution for all the files below /proc: we rely on
the path name and can't check if it's in a procfs filesystem, and that
is not perfect. Moreover, it doesn't work well if we use a link to
access the file or a relative path. If we want a solution managing all
the cases if becomes relatively complex.

From my point of view, all patches are welcome.

For this one:

- don't introduce it as security fix but as a bug fix
- propose a test case and show your fix really fixes it
- you should use do_openat() with execveat() as the exec_path can be
unset in the case of binfmt-misc with the credential flag (search for
AT_EXECFD in QEMU code).

Thanks,
Laurent
diff mbox series

Patch

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 8367cb138d..1a475896a6 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -7504,7 +7504,18 @@  static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
              * before the execve completes and makes it the other
              * program's problem.
              */
-            ret = get_errno(safe_execve(p, argp, envp));
+            {
+                const char *pathname = p;
+                char real_path[PATH_MAX];
+                if (is_proc_myself(pathname, "exe")) {
+                    if (NULL == realpath(exec_path, real_path)) {
+                        ret = get_errno(-1);
+                        goto execve_efault;
+                    }
+                    pathname = real_path;
+                }
+                ret = get_errno(safe_execve(pathname, argp, envp));
+            }
             unlock_user(p, arg1, 0);
 
             goto execve_end;