tags 773318 - moreinfo + patch
thanks
On 2014-12-23 18:15:45 [+0100], Andreas Cadhalpun wrote:
> Hi,
Hi Andreas,
> I think there is a better way than changing the type of frame_end to off_t.
> It is possible to avoid the overflow by reordering the code:
Even better, I like it. The patch at the end of the email is what I
pushed into the wheezy branch for clamav [0]. So after an upload for
stable, we have Wheezy fixed. I have no idea how to close this bug for
unstable since it has to be fixed in a different package. Probably
manually with a comment…
Side question: In case someone has unattended-upgrades running, how does
he get clamd restarted after a libmspack update?
>From a0449d2079c4ba5822e6567ad7094c10108f16cd Mon Sep 17 00:00:00 2001
From: Sebastian Andrzej Siewior <sebast...@breakpoint.cc>
Date: Tue, 23 Dec 2014 21:20:43 +0100
Subject: libmspack: qtmd: fix frame_end overflow
Debian bts #773041, #772891 contains a report of a .cab file which
causes an endless loop.
Eric Sharkey diagnosed the problem as frame_end is 32bit and overflows
and the result the loop makes no progress.
The problem seems that after the overflow, window_posn is larger than
frame_end and therefore we never enter the loop to make progress. But we
still have out_bytes >0 so we don't leave the outer loop either.
Andreas Cadhalpun suggested to instead makeing frame_end 64bit, we could
avoid the overflow by reordering the code the following way:
original, with just out_bytes (without (qtm->o_end - qtm->o_ptr))
| frame_end = window_posn + out_bytes;
| if ((window_posn + frame_todo) < frame_end) {
| frame_end = window_posn + frame_todo;
| }
replace frame_end in "if" with its content (and move the first frame_end
into the else path)
| if ((window_posn + frame_todo) < (window_posn + out_bytes))
| frame_end = window_posn + frame_todo;
| else
| frame_end = window_posn + out_bytes;
remove window_posn from "if" since it is the same both times.
| if (frame_todo < out_bytes)
| frame_end = window_posn + frame_todo;
| else
| frame_end = window_posn + out_bytes;
Andreas added:
|This works, because frame_todo is at most QTM_FRAME_SIZE = 32768.
Suggested-as-patch: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
[sebastian@breakpoint: added patch description]
Signed-off-by: Sebastian Andrzej Siewior <sebast...@breakpoint.cc>
---
libclamav/libmspack-0.4alpha/mspack/qtmd.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/libclamav/libmspack-0.4alpha/mspack/qtmd.c
b/libclamav/libmspack-0.4alpha/mspack/qtmd.c
index 12b27f5608c4..e584aef8e576 100644
--- a/libclamav/libmspack-0.4alpha/mspack/qtmd.c
+++ b/libclamav/libmspack-0.4alpha/mspack/qtmd.c
@@ -296,9 +296,10 @@ int qtmd_decompress(struct qtmd_stream *qtm, off_t
out_bytes) {
/* decode more, up to the number of bytes needed, the frame boundary,
* or the window boundary, whichever comes first */
- frame_end = window_posn + (out_bytes - (qtm->o_end - qtm->o_ptr));
- if ((window_posn + frame_todo) < frame_end) {
+ if (frame_todo < (out_bytes - (qtm->o_end - qtm->o_ptr))) {
frame_end = window_posn + frame_todo;
+ } else {
+ frame_end = window_posn + (out_bytes - (qtm->o_end - qtm->o_ptr));
}
if (frame_end > qtm->window_size) {
frame_end = qtm->window_size;
[0]
https://anonscm.debian.org/cgit/pkg-clamav/clamav.git/tree/debian/patches/0018-libmspack-qtmd-fix-frame_end-overflow.patch?h=wheezy
Sebastian
--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
