Re: [PATCH 1/1] net: zero terminate string with headers in wget_fill_info()

[Jerome Forissier]

On 11/25/24 18:16, Heinrich Schuchardt wrote:
> Commit 2dd076a9c1b4 ("net: wget: integrate struct wget_info into legacy
> wget code") introduced function wget_fill_info() which retrieves the
> headers from the HTTP server response. As we want to parse the string in
> later patches we need to ensure that it is NUL terminated.
> 
> We must further check that wget_info->headers in not NULL.
> Otherwise a crash occurs.
> 
> Fixes: 2dd076a9c1b4 ("net: wget: integrate struct wget_info into legacy wget 
> code")
> Signed-off-by: Heinrich Schuchardt <heinrich.schucha...@canonical.com>
> ---
>  net/wget.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/net/wget.c b/net/wget.c
> index 3bc2522cde5..b290ca1843e 100644
> --- a/net/wget.c
> +++ b/net/wget.c
> @@ -208,8 +208,13 @@ static void wget_fill_info(const uchar *pkt, int hlen)
>       const char *second_space;
>       char *pos, *end;
>  
> -     if (wget_info->headers && hlen < MAX_HTTP_HEADERS_SIZE)
> -             strncpy(wget_info->headers, pkt, hlen);
> +     if (wget_info->headers) {
> +             if (wget_info->headers && hlen < MAX_HTTP_HEADERS_SIZE - 1)

No need to check wget_info->headers twice. Why -1? I can't find where
wget_info->headers is allocated in the original series btw.

> +                     strncpy(wget_info->headers, pkt, hlen);
> +             else
> +                     hlen = 0;
> +             wget_info->headers[hlen] = 0;
> +     }
>  
>       //Get status code
>       first_space = strchr(pkt, ' ');

Thanks,
-- 
Jerome