[Slirp] [PATCH v2] slirp: tftp: restrict relative path access

[Eric Blake]

On 1/2/20 12:07 PM, P J P wrote:
> +-- On Thu, 2 Jan 2020, Eric Blake wrote --+
> | On 1/2/20 5:12 AM, P J P wrote:
> | > Update v2: add conditional compilation of rules with G_OS_WIN32
> | >   https://lists.freedesktop.org/archives/slirp/2020-January/000024.html
> | >
> | > "/../")) {
> | > +#ifdef G_OS_WIN32

The fact that you are using G_OS_WIN32 (a glib macro) makes it seem like 
you really should be relying on glib to do this work for you, rather 
than open-coding it yourself.  glib includes G_IS_DIR_SEPARATOR (which 
matches / on Linux and both / and \\ on Windows), but I'm not readily 
finding a utility function in 
https://developer.gnome.org/glib/stable/glib-Miscellaneous-Utility-Functions.html 
or elsewhere that directly checks a string for a ../ escape.  Even 
something like g_canonicalize_filename() isn't quite enough (you could 
check that the canonical name of the base directory of the request is a 
prefix of the canonical name of the request, but that doesn't rule out 
whether the request traversed beyond the base directory and then back in).

> | > +    if (strstr(req_fname, "..\\") || req_fname[strlen(req_fname) - 1] ==
> | > '\\') {
> | > +#else
> | > +    if (strstr(req_fname, "../") || req_fname[strlen(req_fname) - 1] ==
> | > '/') {
> | > +#endif
> |
> | Note that Windows allows you to pass "foo/../bar" in place of "foo\\..\\bar"
> | and still resolves to the same file, which means your fix is incomplete.
> 
> Oh, in that case maybe earlier version of the patch, without conditional
> rules, would work better?

Maybe:
if (
#if G_OS_WIN32
     strstr(req_fname, "..\\") ||
     req_fname[strlen(req_fname) - 1] == '\\' ||
#endif
     strstr(req_fname, "../") ||
     req_fname[strlen(req_fname) - 1] == '/')

although I'm not a fan of #if mid-expression.

I wish glib would make this easier, as the need to sanitize against 
escapes from the base directory is a common problem.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org