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

[Eric Blake]

On 1/2/20 5:12 AM, P J P 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 | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 
> Update v2: add conditional compilation of rules with G_OS_WIN32
>    -> https://lists.freedesktop.org/archives/slirp/2020-January/000024.html
> 
> diff --git a/src/tftp.c b/src/tftp.c
> index 093c2e0..aaca23e 100644
> --- a/src/tftp.c
> +++ b/src/tftp.c
> @@ -344,8 +344,11 @@ 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, "/../")) {
> +#ifdef G_OS_WIN32
> +    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.


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