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

[P J P]

+-- On Thu, 2 Jan 2020, Marc-André Lureau wrote --+
| >      /* do sanity checks on the filename */
| > -    if (!strncmp(req_fname, "../", 3) ||
| > -        req_fname[strlen(req_fname) - 1] == '/' || strstr(req_fname, "/../")) {
| > +    if (strstr(req_fname, "../") || strstr(req_fname, "..\\")
| > +        || req_fname[strlen(req_fname) - 1] == '/'
| > +        || req_fname[strlen(req_fname) - 1] == '\\') {
| >          tftp_send_error(spt, 2, "Access violation", tp);
| >          return;
| >      }
| 
| Please divide the rule for #ifdef G_OS_WIN32 .. #else ..

  Okay.
 
| You changed the existing rule, and it seems to break some valid cases
| ex: /foo/bar/../blah

  I was wondering about that. The same path can be accessed as '/foo/blah'; 
Not sure what's a valid case for /foo/bar/../blah. I think it'll only help, 
not harm much, to discourage usage like /foo/bar/../blah. Single '../' one can 
still follow, but it also allows instances like
 
   /foo/bar/blah1/blah2/../../../bar1/../bar2/../bar/blah1/file.txt


Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D