[bug #54126] Wget keeps crashing in Windows sometimes when the filename is large enough to scroll it

[Vyacheslav]

Follow-up Comment #6, bug #54126 (project wget):

[comment #5 комментарий №5:]

> 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.

that build must success, later i will try build, BUT ???

Fix buffer overflows in progress 'bar' code
<https://gitlab.com/gnuwget/wget/commit/bbbbe1c6db923dfabc50f8985ee5d994a4be0a1f>
#define BUF_LEN (bp->width * 2 + 100)
must fix overflow without disabling "filename scrolling", and of course,
disabled scrolling will fix problem, but *why ignored wrong
cols_to_bytes()???* With patched cols_to_bytes no needed rewrite 'filename
scroling', it works as expected. With all respect it is not solution (when
USE_NLS_PROGRESS_BAR defined all works fine), May be there is an other reason
for it?

let's: strlen(bp->f_download)=52, offset_bytes=49, bp->width=139,
MAX_FILENAME_COLS = 34
at line 988:
 
bytes_in_filename = cols_to_bytes (bp->f_download + offset_bytes,
MAX_FILENAME_COLS - padding, *cols_ret)

bytes_in_filename will be 34 (MAX_FILENAME_COLS, padding is 0 [padding can not
be negative]), but actually bytes_in_filename is 3
Further, at line 992:
 
memcpy (p, bp->f_download + offset_bytes, bytes_in_filename); 

we are "happy" to copy the rest of the string, including termination '\0' and
a few more bytes after the end of the string, this is not fatal yet  (just the
reason for repeating the last letter), but the beginning of this.
bad thing happens at line 1161:

  padding = bp->width - count_cols (bp->buffer);
  assert (padding >= 0 && "Padding length became non-positive!");
  padding = padding > 0 ? padding : 0;
  memset (p, ' ', padding);
  p += padding;
  *p = '\0';

padding will be too long (136), because in bp->buffer we have '\0' at offset
3, therefore memset causes buffer overflow (width = 139, we are almost at the
end position, buffer = 139 + 100), 
an overflow of 32 bytes (see bar_log_XXX.log in file #48068), and will be
blessed this process by the memory allocation manager and thread context
switcher, and let them be lucky in this (often they got lucky). For what
purposes wget and msvcrt allocate memory in this block ??? in most cases for
the file * output structure and output buffer. Mostly crashes occur when 
RtlFreeHeap is called, after free or fclose.

thanks fflush is called every time after fwrite (retr.c) or otherwise time to
time we can got broken downloaded file.
see attached simple_write_test file in (file #48068) (inexplicable
"\32\32\32\32\32\0" or "\32\32\32\32\32"...,  suspect part of
simple_write_test stream buffer was owerwriten, also an interesting address in
the crash report 0x61616161 "aaaa"). A lot of interesting things can happen as
a result of a buffer overflow...

    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?54126>

_______________________________________________
  Сообщение отправлено по Savannah
  https://savannah.gnu.org/