From: Paul Jackson , Paul Jackson xdelta3 (version 3.0.0) has numerous sprintf and strcpy calls that write into 32 byte char buffers that are on the stack or are static allocations. With sufficiently large files, these strings can overflow the 32 char buffers, and some recent gnu libc versions will detect this and immediately abort with the error "*** buffer overflow detected ***" The first patch in this series increased these buffers from 32 to 48 bytes. This patch, the second in the series, replaces sprintf calls with snprintf, to safely avoid overflowing these stack buffers in all cases. This change necessitated changing the main_format_bcnt() and main_format_millis() API's, to pass the size of the buffer. The third patch will fix a hang caused by not properly closing and flushing pipes. --- xdelta3-blkcache.h | 12 ++++----- xdelta3-main.h | 64 ++++++++++++++++++++++++++--------------------------- 2 files changed, 38 insertions(+), 38 deletions(-) --- xdelta3.0.0.orig/xdelta3-blkcache.h 2012-03-26 23:06:12.280521538 -0500 +++ xdelta3.0.0/xdelta3-blkcache.h 2012-03-26 23:06:47.049599301 -0500 @@ -241,27 +241,27 @@ main_set_source (xd3_stream *stream, xd3 if (sfile->size_known) { - sprintf (srcszbuf, "source size %s [%"Q"u]", - main_format_bcnt (source_size, srccntbuf), + snprintf (srcszbuf, sizeof(srcszbuf), "source size %s [%"Q"u]", + main_format_bcnt (source_size, srccntbuf, sizeof(srccntbuf)), source_size); } else { - strcpy(srcszbuf, "source size unknown"); + strncpy(srcszbuf, "source size unknown", sizeof(srcszbuf)); } nbufs[0] = 0; if (option_verbose > 1) { - sprintf(nbufs, " #bufs %u", lru_size); + snprintf(nbufs, sizeof(nbufs), " #bufs %u", lru_size); } XPR(NT "source %s %s blksize %s window %s%s%s\n", sfile->filename, srcszbuf, - main_format_bcnt (blksize, blkszbuf), - main_format_bcnt (option_srcwinsz, winszbuf), + main_format_bcnt (blksize, blkszbuf, sizeof(blkszbuf)), + main_format_bcnt (option_srcwinsz, winszbuf, sizeof(winszbuf)), nbufs, do_src_fifo ? " (FIFO)" : ""); } --- xdelta3.0.0.orig/xdelta3-main.h 2012-03-26 23:06:12.296522032 -0500 +++ xdelta3.0.0/xdelta3-main.h 2012-03-26 23:22:32.255191072 -0500 @@ -354,7 +354,7 @@ static int main_read_primary_input (main usize_t size, usize_t *nread); -static const char* main_format_bcnt (xoff_t r, char *buf); +static const char* main_format_bcnt (xoff_t r, char *buf, int szbuf); static int main_help (void); /* The code in xdelta3-blk.h is essentially part of this unit, see @@ -576,7 +576,7 @@ get_millisecs_since (void) } static const char* -main_format_bcnt (xoff_t r, char *buf) +main_format_bcnt (xoff_t r, char *buf, int szbuf) { static const char* fmts[] = { "B", "KiB", "MiB", "GiB", "TiB", "PiB", "EiB" }; usize_t i; @@ -587,25 +587,25 @@ main_format_bcnt (xoff_t r, char *buf) if (r == 0) { - sprintf (buf, "0 %s", fmts[i]); + snprintf (buf, szbuf, "0 %s", fmts[i]); return buf; } if (r >= 1 && r < 10) { - sprintf (buf, "%.2f %s", (double) r, fmts[i]); + snprintf (buf, szbuf, "%.2f %s", (double) r, fmts[i]); return buf; } if (r >= 10 && r < 100) { - sprintf (buf, "%.1f %s", (double) r, fmts[i]); + snprintf (buf, szbuf, "%.1f %s", (double) r, fmts[i]); return buf; } if (r >= 100 && r < 1000) { - sprintf (buf, "%"Q"u %s", r, fmts[i]); + snprintf (buf, szbuf, "%"Q"u %s", r, fmts[i]); return buf; } @@ -613,13 +613,13 @@ main_format_bcnt (xoff_t r, char *buf) if (new_r < 10) { - sprintf (buf, "%.2f %s", (double) r / 1024.0, fmts[i + 1]); + snprintf (buf, szbuf, "%.2f %s", (double) r / 1024.0, fmts[i + 1]); return buf; } if (new_r < 100) { - sprintf (buf, "%.1f %s", (double) r / 1024.0, fmts[i + 1]); + snprintf (buf, szbuf, "%.1f %s", (double) r / 1024.0, fmts[i + 1]); return buf; } @@ -630,22 +630,22 @@ main_format_bcnt (xoff_t r, char *buf) } static char* -main_format_rate (xoff_t bytes, long millis, char *buf) +main_format_rate (xoff_t bytes, long millis, char *buf, int szbuf) { xoff_t r = (xoff_t)(1.0 * bytes / (1.0 * millis / 1000.0)); static char lbuf[48]; - main_format_bcnt (r, lbuf); - sprintf (buf, "%s/s", lbuf); + main_format_bcnt (r, lbuf, sizeof(lbuf)); + snprintf (buf, szbuf, "%s/s", lbuf); return buf; } static char* -main_format_millis (long millis, char *buf) +main_format_millis (long millis, char *buf, int szbuf) { - if (millis < 1000) { sprintf (buf, "%lu ms", millis); } - else if (millis < 10000) { sprintf (buf, "%.1f sec", millis / 1000.0); } - else { sprintf (buf, "%lu sec", millis / 1000L); } + if (millis < 1000) { snprintf (buf, szbuf, "%lu ms", millis); } + else if (millis < 10000) { snprintf (buf, szbuf, "%.1f sec", millis / 1000.0); } + else { snprintf (buf, szbuf, "%lu sec", millis / 1000L); } return buf; } @@ -2739,11 +2739,11 @@ main_set_appheader (xd3_stream *stream, if (sfile->filename == NULL) { - sprintf ((char*)appheader_used, "%s/%s", iname, icomp); + snprintf ((char*)appheader_used, len, "%s/%s", iname, icomp); } else { - sprintf ((char*)appheader_used, "%s/%s/%s/%s", + snprintf ((char*)appheader_used, len, "%s/%s/%s/%s", iname, icomp, sname, scomp); } } @@ -2967,7 +2967,7 @@ main_get_winsize (main_file *ifile) { { XPR(NT "input %s window size %s\n", ifile->filename, - main_format_bcnt (size, iszbuf)); + main_format_bcnt (size, iszbuf, sizeof(iszbuf))); } return size; @@ -3345,25 +3345,25 @@ main_input (xd3_cmd cmd, XPR(NT "%"Q"u: in %s (%s): out %s (%s): " "total in %s: out %s: %s: srcpos %s\n", stream.current_window, - main_format_bcnt (this_read, rdb), - main_format_rate (this_read, millis, rrateavg), - main_format_bcnt (this_write, wdb), - main_format_rate (this_write, millis, wrateavg), - main_format_bcnt (stream.total_in, trdb), - main_format_bcnt (stream.total_out, twdb), - main_format_millis (millis, tm), - main_format_bcnt (sfile->source_position, srcpos)); + main_format_bcnt (this_read, rdb, sizeof(rdb)), + main_format_rate (this_read, millis, rrateavg, sizeof(rrateavg)), + main_format_bcnt (this_write, wdb, sizeof(wdb)), + main_format_rate (this_write, millis, wrateavg, sizeof(wrateavg)), + main_format_bcnt (stream.total_in, trdb, sizeof(trdb)), + main_format_bcnt (stream.total_out, twdb, sizeof(twdb)), + main_format_millis (millis, tm, sizeof(tm)), + main_format_bcnt (sfile->source_position, srcpos, sizeof(srcpos))); } else { XPR(NT "%"Q"u: in %s: out %s: total in %s: " "out %s: %s\n", stream.current_window, - main_format_bcnt (this_read, rdb), - main_format_bcnt (this_write, wdb), - main_format_bcnt (stream.total_in, trdb), - main_format_bcnt (stream.total_out, twdb), - main_format_millis (millis, tm)); + main_format_bcnt (this_read, rdb, sizeof(rdb)), + main_format_bcnt (this_write, wdb, sizeof(wdb)), + main_format_bcnt (stream.total_in, trdb, sizeof(trdb)), + main_format_bcnt (stream.total_out, twdb, sizeof(twdb)), + main_format_millis (millis, tm, sizeof(tm))); } } } @@ -3465,7 +3465,7 @@ done: xoff_t nwrite = ofile != NULL ? ofile->nwrite : 0; XPR(NT "finished in %s; input %"Q"u output %"Q"u bytes (%0.2f%%)\n", - main_format_millis (end_time - start_time, tm), + main_format_millis (end_time - start_time, tm, sizeof(tm)), ifile->nread, nwrite, 100.0 * nwrite / ifile->nread); }