[Qemu-devel] [PATCH 0/1] Handle /proc/self/exe in execve

[Qemu-devel] [PATCH 0/1] Handle /proc/self/exe in execve

[dion]

From: Olivier Dion <dion@linutronix.de>

When the emulated process try to execve itself through /proc/self/exe,
QEMU user will be executed instead of the process.

The following short program demonstrated that:
----------------------------------------------------------------------
#include <stdio.h>
#include <string.h>
#include <unistd.h>


static char *ARGV0 = "STOP";
static char *ARGV1 = "-this-is-not-an-option";


int main(int argc, char *argv[], char *environ[])
{
        (void)argc;
        if (strcmp(argv[0], ARGV0) == 0)
                return 0;
        argv[0] = ARGV0;
        argv[1] = ARGV1;
        execve("/proc/self/exe", (char **const)argv,
               (char **const)environ);
        perror("execve");
        return 1;
}
----------------------------------------------------------------------

Will output:
----------------------------------------------------------------------
qemu: unknown option 'this-is-not-an-option'
----------------------------------------------------------------------

Olivier Dion (1):
  linux-user:  Handle /proc/self/exe in syscall execve

 linux-user/syscall.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

-- 
2.22.0

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

[dion]

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(-)

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;
-- 
2.22.0

Re: [Qemu-devel] [PATCH 1/1] linux-user: Handle /proc/self/exe in syscall execve

[Laurent Vivier]

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

Re: [Qemu-devel] [PATCH 1/1] linux-user: Handle /proc/self/exe in syscall execve

[Olivier Dion]

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.

-- 
Olivier Dion
Polymtl

Re: [Qemu-devel] [PATCH 1/1] linux-user: Handle /proc/self/exe in syscall execve

[Laurent Vivier]

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

[Qemu-devel] [PATCH v2 0/1] Handle /proc/self/exe in execve

[Olivier Dion]

* Changes from v1

  - Introduce the patch as a bug fix, rather than a security fix
  - Use do_openat and safe_execveat instead of copying exec_path
  - Extensive test case example
  
* Test case

  I will present a short program that demonstrated the bug, i.e. what
  is the expected behavior and what really happens.  Then, I will
  explain how this patch fixes this bug.

** The program

   -------------------------------------------------------------------
   #include <errno.h>
   #include <string.h>
   #include <unistd.h>


   static char *ARG0 = "STOP";
   static char *ARG1 = "-this-is-not-an-option";


   int main(int argc, char *argv[], char *envp[])
   {
           (void)argc;
           if (0 == strcmp(argv[0], ARG0))
                   return 0;
           argv[0] = ARG0;
           argv[1] = ARG1;
           execve("/proc/self/exe", 
                  (char **const)argv,
                  (char **const)envp);
           return errno;
   }
   -------------------------------------------------------------------

   Note that in every cases, this program should be run with at least
   one argument, so that argv[1] points to something.

*** Expected behavior

   This program when run normally, i.e. without an emulator or with
   this patch applied, will run two times.  The first time, it will
   change its argv[0] and argv[1] and recursively call itself.  The
   second time, it will stop at the string comparaison between argv[0]
   and the sentinel ARG0, returning 0.  Thus, we expect the program to
   finish with error code 0 and nothing is printed to stdout&stderr.

*** What really happens

   When emulated by qemu-user, this program will fail to call itself
   recursively and will instead call qemu-user.  This is where ARG1
   becomes useful.  It's indeed set to an option that is not supported
   by qemu-user, and thus we expected two things

       1) A message will be printed to stdout&|stderr
       2) A error code different from 0 will be returned

   For example, I get the following output with error code 1
   -------------------------------------------------------------------
   qemu: unknown option 'this-is-not-an-option'
   -------------------------------------------------------------------

*** Automated testing

    The following is a quick bash script that demonstrates how to use
    this test case.  I suppose here that qemu-user is the correct
    emulator for the arch of the compiled program a.out.
    ------------------------------------------------------------------
    #!/bin/bash
    out=$(qemu-user ./a.out foo)
    ret=0
    if [[ $out != "" || $? != 0 ]]; then
        ret=1
    fi
    exit $ret
    ------------------------------------------------------------------

* Fixing the bug

   This patch introduces the use of safe_execveat instead of
   safe_execve for the emulation of execve.  By using the do_openat
   function, we ensure that the executable file descriptor is really
   the one the user wants.


Olivier Dion (1):
  Handle /proc/self/exe in syscall execve

 linux-user/syscall.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

-- 
2.23.0

[Qemu-devel] [PATCH v2 1/1] Handle /proc/self/exe in syscall execve

[Olivier Dion]

If not handled, QEMU will execve itself instead of the emulated
process.  The function do_openat already solves the /proc/self/exe
problem, so we can use it to get the executable file descriptor.  We
then make a safe call to execveat.

Note that safe_execve has been replaced by safe_execveat, since the
former is now useless.

Signed-off-by: Olivier Dion <olivier.dion@polymtl.ca>
---
 linux-user/syscall.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index e2af3c1494..68340bcb67 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -736,7 +736,7 @@ safe_syscall4(pid_t, wait4, pid_t, pid, int *, status, int, options, \
               struct rusage *, rusage)
 safe_syscall5(int, waitid, idtype_t, idtype, id_t, id, siginfo_t *, infop, \
               int, options, struct rusage *, rusage)
-safe_syscall3(int, execve, const char *, filename, char **, argv, char **, envp)
+safe_syscall5(int, execveat, int, dirfd, const char *, pathname, char **, argv, char **, envp, int, flags)
 safe_syscall6(int, pselect6, int, nfds, fd_set *, readfds, fd_set *, writefds, \
               fd_set *, exceptfds, struct timespec *, timeout, void *, sig)
 safe_syscall5(int, ppoll, struct pollfd *, ufds, unsigned int, nfds,
@@ -7507,8 +7507,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));
-            unlock_user(p, arg1, 0);
+            {
+                int execfd = get_errno(do_openat(cpu_env, AT_FDCWD, p, O_PATH, 0));
+                unlock_user(p, arg1, 0);
+                if (is_error(execfd)) {
+                    ret = execfd;
+                    goto execve_end;
+                }
+                ret = get_errno(safe_execveat(execfd, "",
+                                              argp, envp,
+                                              AT_EMPTY_PATH));
+                close(execfd);
+            }
 
             goto execve_end;
 
-- 
2.23.0

Re: [Qemu-devel] [PATCH v2 0/1] Handle /proc/self/exe in execve

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190916155545.29928-1-olivier.dion@polymtl.ca/



Hi,

This series failed build test on FreeBSD host. Please find the details below.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
# Testing script will be invoked under the git checkout with
# HEAD pointing to a commit that has the patches applied on top of "base"
# branch
if qemu-system-x86_64 --help >/dev/null 2>&1; then
  QEMU=qemu-system-x86_64
elif /usr/libexec/qemu-kvm --help >/dev/null 2>&1; then
  QEMU=/usr/libexec/qemu-kvm
else
  exit 1
fi
make vm-build-freebsd J=21 QEMU=$QEMU
exit 0
=== TEST SCRIPT END ===




The full log is available at
http://patchew.org/logs/20190916155545.29928-1-olivier.dion@polymtl.ca/testing.FreeBSD/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com