[U-Boot] [PATCH] cmd: fastboot: Validate user input

[Simon Glass]

Hi Sam,

On 26 June 2018 at 10:20, Sam Protsenko <semen.protsenko at linaro.org> wrote:
> In case when user provides '-' as USB controller index, like this:
>
>     => fastboot -
>
> data abort occurs in strcmp() function in do_fastboot(), here:
>
>     if (!strcmp(argv[1], "udp"))
>
> That's because argv[1] is NULL when user types in the '-', and null
> pointer dereference occurs in strcmp() (which is ok according to C
> standard specification). So we must validate user input to prevent such
> behavior.
>
> While at it, check also the result of strtoul() function and handle
> error cases properly.
>
> Signed-off-by: Sam Protsenko <semen.protsenko at linaro.org>
> ---
>  cmd/fastboot.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)

Reviewed-by: Simon Glass <sjg at chromium.org>

Question below.

>
> diff --git a/cmd/fastboot.c b/cmd/fastboot.c
> index e6ae0570d5..dab686eef4 100644
> --- a/cmd/fastboot.c
> +++ b/cmd/fastboot.c
> @@ -38,13 +38,18 @@ static int do_fastboot_usb(int argc, char *const argv[],
>  #if CONFIG_IS_ENABLED(USB_FUNCTION_FASTBOOT)
>         int controller_index;
>         char *usb_controller;
> +       char *endp;
>         int ret;
>
>         if (argc < 2)
>                 return CMD_RET_USAGE;
>
>         usb_controller = argv[1];
> -       controller_index = simple_strtoul(usb_controller, NULL, 0);
> +       controller_index = simple_strtoul(usb_controller, &endp, 0);
> +       if (*endp != '\0') {
> +               pr_err("Error: Wrong USB controller index format\n");
> +               return CMD_RET_FAILURE;
> +       }
>
>         ret = board_usb_init(controller_index, USB_INIT_DEVICE);
>         if (ret) {
> @@ -120,6 +125,12 @@ NXTARG:
>                 ;
>         }
>
> +       /* Handle case when USB controller param is just '-' */
> +       if (!argv[1]) {

Can you check argc instead? I think that is nicer.

> +               pr_err("Error: Incorrect USB controller index\n");
> +               return CMD_RET_USAGE;
> +       }
> +
>         fastboot_init((void *)buf_addr, buf_size);
>
>         if (!strcmp(argv[1], "udp"))
> --
> 2.17.1
>