Re: [PATCH] tftp: Normalize slashes in tftp paths

[Daniel Kiper]

On Thu, Oct 31, 2019 at 11:33:39AM +0100, Javier Martinez Canillas wrote:
> From: Lenny Szubowicz <address@hidden>
>
> Some tftp servers do not handle multiple consecutive slashes correctly;
> this patch avoids sending tftp requests with non-normalized paths.
>
> Signed-off-by: Lenny Szubowicz <address@hidden>
> Signed-off-by: Mark Salter <address@hidden>
> Signed-off-by: Javier Martinez Canillas <address@hidden>

Reviewed-by: Daniel Kiper <address@hidden>

Except some nitpicks which I fix before committing...

> ---
>
>  grub-core/net/tftp.c | 28 +++++++++++++++++++++++++---
>  1 file changed, 25 insertions(+), 3 deletions(-)
>
> diff --git grub-core/net/tftp.c grub-core/net/tftp.c
> index 7d90bf66e76..6dbb9cdbb7a 100644
> --- grub-core/net/tftp.c
> +++ grub-core/net/tftp.c
> @@ -300,6 +300,25 @@ destroy_pq (tftp_data_t data)
>    grub_priority_queue_destroy (data->pq);
>  }
>
> +/* Create a normalized copy of the filename.
> +   Compress any string of consecutive forward slashes to a single forward
> +   slash. */

https://www.gnu.org/software/grub/manual/grub-dev/grub-dev.html#Multi_002dLine-Comments

> +static void
> +grub_normalize_filename (char *normalized, const char *filename)
> +{
> +  char *dest = normalized;
> +  const char *src = filename;
> +
> +  while (*src != '\0')
> +    {
> +      if (src[0] == '/' && src[1] == '/')
> +        src++;
> +      else
> +        *dest++ = *src++;
> +    }
> +  *dest = '\0';
> +}
> +
>  static grub_err_t
>  tftp_open (struct grub_file *file, const char *filename)
>  {
> @@ -337,9 +356,12 @@ tftp_open (struct grub_file *file, const char *filename)
>    rrqlen = 0;
>
>    tftph->opcode = grub_cpu_to_be16_compile_time (TFTP_RRQ);
> -  grub_strcpy (rrq, filename);
> -  rrqlen += grub_strlen (filename) + 1;
> -  rrq += grub_strlen (filename) + 1;
> +
> +  /* Copy and normalize the filename to work-around issues on some tftp
> +     servers when file names are being matched for remapping. */

Ditto.

> +  grub_normalize_filename (rrq, filename);
> +  rrqlen += grub_strlen (rrq) + 1;
> +  rrq += grub_strlen (rrq) + 1;
>
>    grub_strcpy (rrq, "octet");
>    rrqlen += grub_strlen ("octet") + 1;

Daniel