Re: wget 1.19.4 has a buffer overflow vulnerability when formating total download time
Hi JunDong Xie,
today I fixed a bunch of issues found by fuzzing in the progress (bar +
dot) code. The 'filename scrolling' part of the progress bar needed to
be disabled and likely has to be rewritten.
The changes are in branch 'tmp-progress-fuzzer' in
https://gitlab.com/gnuwget/wget.git.
Is it possible that you build wget from that branch and test it ?
If you report success, the changes can be pushed to master.
Regards, Tim
On 11.12.19 17:18, Tim Rühsen wrote:
> 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 写道：
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
Re: wget 1.19.4 has a buffer overflow vulnerability when formating total download time
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 写道：
>>>
>>> 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
Re: wget 1.19.4 has a buffer overflow vulnerability when formating total download time
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 写道：
>>
>> 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
Re: wget 1.19.4 has a buffer overflow vulnerability when formating total download time
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
