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

[Tim Rühsen]

Hi,

today I wrote a fuzzer to test the progress code. After fixing several
buffer and integer overflows even more and more pop up. It looks like
the progress code has never been thoroughly tested for non-ASCII
locales. And this is just the "bar" progress code.

Fixing this needs more time than I currently have.
If someone wants to work on it, I would share the fuzzer plus all the
instructions to build and use it.

Regards, Tim

On 12/11/19 10:00 AM, Tim Rühsen wrote:
> 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