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

[Marc-André Lureau]

Hi

On Mon, Dec 30, 2019 at 5:00 PM P J P <ppandit at redhat.com> wrote:
>
> From: Prasad J Pandit <pjp at fedoraproject.org>
>
> tftp restricts relative or directory path access on Linux systems.
> Apply same restrictions on Windows systems too. It helps to avoid
> directory traversal issue.
>
> Reported-by: Peter Maydell <peter.maydell at linaro.org>
> Signed-off-by: Prasad J Pandit <pjp at fedoraproject.org>
> ---
>  src/tftp.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/src/tftp.c b/src/tftp.c
> index 093c2e0..55d9353 100644
> --- a/src/tftp.c
> +++ b/src/tftp.c
> @@ -344,8 +344,9 @@ static void tftp_handle_rrq(Slirp *slirp, struct sockaddr_storage *srcsas,
>      k += 6; /* skipping octet */
>
>      /* 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 ..

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

This traversal problem is so common, I wonder if there are known good
practices...

> --
> 2.24.1
>
> _______________________________________________
> Slirp mailing list
> Slirp at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/slirp



-- 
Marc-André Lureau