qemu-trivial
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-trivial] [PATCH v2] linux-user: fix is_proc_myself to check th


From: Zach Riggle
Subject: Re: [Qemu-trivial] [PATCH v2] linux-user: fix is_proc_myself to check the paths via realpath
Date: Fri, 27 Oct 2017 14:07:24 -0500

Another case that may be more relevant for general QEMU use, is that the current code fails if the software under test has poor path-handling code.  For example, any of

- //proc/self/maps
- /proc//self/maps
- /proc/self//maps

Will all return the non-emulated results.  Those examples are just path canonicalization issues and could be resolved with e.g. canonicalize_file_name, but I'm not sure if QEMU allows GNU extensions -- and realpath() is portable.




Zach Riggle


On Fri, Oct 27, 2017 at 12:05 PM, Zach Riggle <address@hidden> wrote:
The symlink was just an easy test case.  Doing cd proc && cat ./self/maps will achieve the same thing.

One instance where this matters is for testing IoT device firmware exploits, where one might do a GET request against ../../../../../../proc/self/maps in order to bypass ASLR.  Currently, this will return the memory layout of QEMU, not the emulated memory layout of the software under test.


Zach Riggle


On Fri, Oct 27, 2017 at 4:36 AM, Riku Voipio <address@hidden> wrote:
On Thu, Oct 26, 2017 at 04:06:22PM -0500, Zach Riggle wrote:
> Friendly ping :)
>
> I've updated the patch with v2 which addresses the style issue

I'll have a look at it soon.

>
> *Zach Riggle*
>
> On Tue, Oct 24, 2017 at 10:34 PM, Zach Riggle <address@hidden> wrote:
>
> > Previously, it was possible to get a handle to the "real" /proc/self/mem
> > by creating a symlink to it and opening the symlink, or opening e.g.
> > "./mem" after chdir'ing to "/proc/self"

When is this a problem? Symlinking to /proc/self seems to be a quite weird usecase.

> >
> >     $ ln -s /proc/self self
> >     $ cat self/maps
> >     60000000-602bc000 r-xp 00000000 fc:01 270375
> >    /usr/bin/qemu-arm-static
> >     604bc000-6050f000 rw-p 002bc000 fc:01 270375
> >    /usr/bin/qemu-arm-static
> >     ...
> >
> > Signed-off-by: Zach Riggle <address@hidden>
> > ---
> >  linux-user/syscall.c | 47 ++++++++++++++++++++++++++++-------------------
> >  1 file changed, 28 insertions(+), 19 deletions(-)
> >
> > diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> > index 9bf901fa11..6c1f28a1f7 100644
> > --- a/linux-user/syscall.c
> > +++ b/linux-user/syscall.c
> > @@ -7496,26 +7496,35 @@ static int open_self_auxv(void *cpu_env, int fd)
> >
> >  static int is_proc_myself(const char *filename, const char *entry)
> >  {
> > -    if (!strncmp(filename, "/proc/", strlen("/proc/"))) {
> > -        filename += strlen("/proc/");
> > -        if (!strncmp(filename, "self/", strlen("self/"))) {
> > -            filename += strlen("self/");
> > -        } else if (*filename >= '1' && *filename <= '9') {
> > -            char myself[80];
> > -            snprintf(myself, sizeof(myself), "%d/", getpid());
> > -            if (!strncmp(filename, myself, strlen(myself))) {
> > -                filename += strlen(myself);
> > -            } else {
> > -                return 0;
> > -            }
> > -        } else {
> > -            return 0;
> > -        }
> > -        if (!strcmp(filename, entry)) {
> > -            return 1;
> > -        }
> > +    char proc_self_entry[PATH_MAX + 1];
> > +    char proc_self_entry_realpath[PATH_MAX + 1];
> > +    char filename_realpath[PATH_MAX + 1];
> > +
> > +    if (PATH_MAX < snprintf(proc_self_entry,
> > +                            sizeof(proc_self_entry),
> > +                            "/proc/self/%s",
> > +                            entry)) {
> > +        /* Full path to "entry" is too long to fit in the buffer */
> > +        return 0;
> >      }
> > -    return 0;
> > +
> > +    if (!realpath(filename, filename_realpath)) {
> > +        /* File does not exist, or can't be canonicalized */
> > +        return 0;
> > +    }
> > +
> > +    if (!realpath(proc_self_entry, proc_self_entry_realpath)) {
> > +        /* Procfs entry does not exist */
> > +        return 0;
> > +    }
> > +
> > +    if (strcmp(filename_realpath, proc_self_entry_realpath) != 0) {
> > +        /* Paths are different */
> > +        return 0;
> > +    }
> > +
> > +    /* filename refers to /proc/self/<entry> */
> > +    return 1;
> >  }
> >
> >  #if defined(HOST_WORDS_BIGENDIAN) != defined(TARGET_WORDS_BIGENDIAN)
> > --
> > 2.14.3
> >
> >



reply via email to

[Prev in Thread] Current Thread [Next in Thread]