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

* [U-Boot] [PATCH v2] cmd: fastboot: Validate user input
@ 2018-06-29 18:59 Sam Protsenko
  2018-06-30  4:20 ` Simon Glass
  0 siblings, 1 reply; 6+ messages in thread
From: Sam Protsenko @ 2018-06-29 18:59 UTC (permalink / raw)
  To: u-boot

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"))

(tested on BeagleBone Black).

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@linaro.org>
---
Changes for v2:
  - replace argv check with argc check
  - add mentioning of testing platform in commit message

 cmd/fastboot.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/cmd/fastboot.c b/cmd/fastboot.c
index e6ae0570d5..ae3a5f627f 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 (argc == 1) {
+		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

^ permalink raw reply related	[flat|nested] 6+ messages in thread