| Message ID | 20241008094646.4052174-1-jerome.forissier@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | net: wget: check strict_strtoul() return value | expand |
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 >
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.
On 10/27/24 18:32, Tom Rini wrote: > 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. > Yeah. The strict_strtoul() call is wrong in the first place. Replaced with: https://lore.kernel.org/u-boot/20241105110849.41348-1-jerome.forissier@linaro.org/T/#u Thanks,
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);
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(-)