diff mbox series

net: wget: check strict_strtoul() return value

Message ID 20241008094646.4052174-1-jerome.forissier@linaro.org
State New
Headers show
Series net: wget: check strict_strtoul() return value | expand

Commit Message

Jérôme Forissier Oct. 8, 2024, 9:46 a.m. UTC
Check the return value of strict_strtoul() when processing the
Content-Length header as recommended by Coverity [1].

[1] https://lists.denx.de/pipermail/u-boot/2024-October/567050.html

Reported-by: Coverity (CID 510464)
Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
---
 net/wget.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Simon Glass Oct. 9, 2024, 1:57 a.m. UTC | #1
On Tue, 8 Oct 2024 at 03:47, Jerome Forissier
<jerome.forissier@linaro.org> wrote:
>
> Check the return value of strict_strtoul() when processing the
> Content-Length header as recommended by Coverity [1].
>
> [1] https://lists.denx.de/pipermail/u-boot/2024-October/567050.html
>
> Reported-by: Coverity (CID 510464)
> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
> ---
>  net/wget.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>

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


> diff --git a/net/wget.c b/net/wget.c
> index b4251e0f293..a3821495e03 100644
> --- a/net/wget.c
> +++ b/net/wget.c
> @@ -256,7 +256,12 @@ static void wget_connected(uchar *pkt, unsigned int tcp_seq_num,
>                                 content_length = -1;
>                         } else {
>                                 pos += sizeof(content_len) + 2;
> -                               strict_strtoul(pos, 10, &content_length);
> +                               if (strict_strtoul(pos, 10, &content_length) < 0) {
> +                                       wget_loop_state = NETLOOP_FAIL;
> +                                       wget_fail("wget: bad Content-Length\n", tcp_seq_num, tcp_ack_num, action);
> +                                       net_set_state(NETLOOP_FAIL);
> +                                       return;
> +                               }
>                                 debug_cond(DEBUG_WGET,
>                                            "wget: Connected Len %lu\n",
>                                            content_length);
> --
> 2.40.1
>
Tom Rini Oct. 27, 2024, 6:32 p.m. UTC | #2
On Tue, Oct 08, 2024 at 11:46:46AM +0200, Jerome Forissier wrote:

> Check the return value of strict_strtoul() when processing the
> Content-Length header as recommended by Coverity [1].
> 
> [1] https://lists.denx.de/pipermail/u-boot/2024-October/567050.html
> 
> Reported-by: Coverity (CID 510464)
> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
>  net/wget.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/net/wget.c b/net/wget.c
> index b4251e0f293..a3821495e03 100644
> --- a/net/wget.c
> +++ b/net/wget.c
> @@ -256,7 +256,12 @@ static void wget_connected(uchar *pkt, unsigned int tcp_seq_num,
>  				content_length = -1;
>  			} else {
>  				pos += sizeof(content_len) + 2;
> -				strict_strtoul(pos, 10, &content_length);
> +				if (strict_strtoul(pos, 10, &content_length) < 0) {
> +					wget_loop_state = NETLOOP_FAIL;
> +					wget_fail("wget: bad Content-Length\n", tcp_seq_num, tcp_ack_num, action);
> +					net_set_state(NETLOOP_FAIL);
> +					return;
> +				}
>  				debug_cond(DEBUG_WGET,
>  					   "wget: Connected Len %lu\n",
>  					   content_length);

This leads to:
U-Boot> wget 200000 EFI/arm64/helloworld.efi
Waiting for Ethernet connection... done.
HTTP/1.0 200 OKwget: Transfer Fail - wget: bad Content-Length

On for example Pi without lwIP enabled. This works otherwise.
diff mbox series

Patch

diff --git a/net/wget.c b/net/wget.c
index b4251e0f293..a3821495e03 100644
--- a/net/wget.c
+++ b/net/wget.c
@@ -256,7 +256,12 @@  static void wget_connected(uchar *pkt, unsigned int tcp_seq_num,
 				content_length = -1;
 			} else {
 				pos += sizeof(content_len) + 2;
-				strict_strtoul(pos, 10, &content_length);
+				if (strict_strtoul(pos, 10, &content_length) < 0) {
+					wget_loop_state = NETLOOP_FAIL;
+					wget_fail("wget: bad Content-Length\n", tcp_seq_num, tcp_ack_num, action);
+					net_set_state(NETLOOP_FAIL);
+					return;
+				}
 				debug_cond(DEBUG_WGET,
 					   "wget: Connected Len %lu\n",
 					   content_length);