Re: wget 1.19.4 has a buffer overflow vulnerability when formating total download time

[Tim Rühsen]

Oh, sorry about that.

You are right - and thanks for your tracing/comments.

The code is really makes some unsafe assumptions. Together with insecure
programming it's the best recipe for memory bugs.

I will try to reproduce the issue with the latest sources.
It would be great if you can open a new issue for this in the meantime.

Thank you, Tim

On 12/11/19 7:44 AM, JunDong Xie wrote:
> Thanks, but I suppose that my issue is different from the post. It is not a 
> long filename related issue, it is related to the display of total download 
> time.  
> Should I open another issue to discuss this problem?
> Regards, dddong
> 
>> 在 2019年12月11日，上午2:59，Tim Rühsen <address@hidden> 写道：
>>
>> Thanks,
>>
>> it's discussed at https://savannah.gnu.org/bugs/?54126. Feel free to add
>> information.
>>
>> Regards, Tim
>>
>> On 10.12.19 08:28, JunDong Xie wrote:
>>> This bug is in progress.c, create_image function. 
>>> ```
>>> else
>>>    {
>>>      /* When the download is done, print the elapsed time.  */
>>>      int nbytes;
>>>      int ncols;
>>>
>>>      /* Note to translators: this should not take up more room than
>>>         available here (6 columns).  Abbreviate if necessary.  */
>>>      strcpy (p, _("    in "));
>>>      nbytes = strlen (p);
>>>      ncols  = count_cols (p); //(1) ncols is 9 in my environment
>>>      bytes_cols_diff = nbytes - ncols;
>>>      if (dl_total_time >= 10)
>>>        ncols += sprintf (p + nbytes, "%s",  eta_to_human_short ((int) 
>>> (dl_total_time + 0.5), false)); //(2) eta_to_human_short may return a 
>>> string like '17m 20s' which length is 7. ncols is 0x10 now. 
>>>      else
>>>        ncols += sprintf (p + nbytes, "%ss", print_decimal (dl_total_time));
>>>      p += ncols + bytes_cols_diff;
>>>      memset (p, ' ', PROGRESS_ETA_LEN - ncols); // (3) PROGRESS_ETA_LEN is 
>>> 15. so the third parameter of memset becomes -1, which cause a buffer 
>>> overflow in heap.
>>>      p += PROGRESS_ETA_LEN - ncols;
>>>    }
>>> ```
>>> when the download is done, wget needs to print the elapsed time. In (1), 
>>> ncols is assigned 9. In (2), the longest length of string returned by 
>>> eta_to_human_short is 7, which causes ncols becomes 0x10. In (3), 
>>> PROGRESS_ETA_LEN - ncols is less than zero and there is no check here. 
>>> memset’s third parameter is an unsigned integer, so it is an integer 
>>> underflow, which causes out-of-bounds write in heap. 
>>>
>>> Below is my wget version.
>>> ```
>>> wget --version           dddong@dddong-vm-ubuntu-18
>>> GNU Wget 1.19.4 在 linux-gnu 上编译。
>>>
>>> -cares +digest -gpgme +https +ipv6 +iri +large-file -metalink +nls
>>> +ntlm +opie +psl +ssl/openssl
>>>
>>> Wgetrc:
>>>    /etc/wgetrc (系统)
>>> locale:
>>>    /usr/share/locale
>>> compile:
>>>    gcc -DHAVE_CONFIG_H -DSYSTEM_WGETRC="/etc/wgetrc"
>>>    -DLOCALEDIR="/usr/share/locale" -I. -I../../src -I../lib
>>>    -I../../lib -Wdate-time -D_FORTIFY_SOURCE=2 -DHAVE_LIBSSL -DNDEBUG
>>>    -g -O2 -fdebug-prefix-map=/build/wget-Xb5Z7Y/wget-1.19.4=.
>>>    -fstack-protector-strong -Wformat -Werror=format-security
>>>    -DNO_SSLv2 -D_FILE_OFFSET_BITS=64 -g -Wall
>>> link:
>>>    gcc -DHAVE_LIBSSL -DNDEBUG -g -O2
>>>    -fdebug-prefix-map=/build/wget-Xb5Z7Y/wget-1.19.4=.
>>>    -fstack-protector-strong -Wformat -Werror=format-security
>>>    -DNO_SSLv2 -D_FILE_OFFSET_BITS=64 -g -Wall -Wl,-Bsymbolic-functions
>>>    -Wl,-z,relro -Wl,-z,now -lpcre -luuid -lidn2 -lssl -lcrypto -lpsl
>>>    ftp-opie.o openssl.o http-ntlm.o ../lib/libgnu.a
>>>
>>> ```
>>>
>>> It is quite annoying me when download large files which often causes wget 
>>> to crash. Hope for your reply! 
>>>
>>
>

signature.asc
Description: OpenPGP digital signature