Processed: Re: Bug#773041: Bug#773318: clamav dies/hangs
Processing control commands: tag 773041 + pending Bug #773041 [libmspack0] libmspack: hangs on a crafted CAB file Added tag(s) pending. -- 773041: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=773041 Debian Bug Tracking System Contact ow...@bugs.debian.org with problems -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#773041: Bug#773318: clamav dies/hangs
Control: tag 773041 + pending Coin, Thanks a lot Andreas. Have a pleasant end of year time. -- Marc Dequènes -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#773041: Bug#773318: clamav dies/hangs
Hi,
On 22.12.2014 22:52, Sebastian Andrzej Siewior wrote:
On 2014-12-22 02:52:02 [+0100], Marc Dequènes (duck) wrote:
I can upload this simple fix quickly, nevertheless i did not have time to
proofread it. Any comment?
I plan to add the following patch to clamav. I added a small comment
why we have the busy loop there. So far it looks like a good idea. The
only problem is that we need off_t beeing 64bit (LFS) or it won't work
on 32bit. No problem on Debian side…
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:
--- libmspack-0.4.orig/mspack/qtmd.c
+++ libmspack-0.4/mspack/qtmd.c
@@ -296,10 +296,12 @@ int qtmd_decompress(struct qtmd_stream *
/* 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;
}
This works, because frame_todo is at most QTM_FRAME_SIZE = 32768.
Merry Christmas,
Andreas
--
To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#773041: Bug#773318: clamav dies/hangs
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-rc-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Processed: Re: Bug#773041: Bug#773318: clamav dies/hangs
Processing commands for cont...@bugs.debian.org: tags 773318 - moreinfo + patch Bug #773318 [clamav-daemon] clamav dies/hangs Removed tag(s) moreinfo. Bug #773318 [clamav-daemon] clamav dies/hangs Added tag(s) patch. thanks Stopping processing here. Please contact me if you need assistance. -- 773318: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=773318 Debian Bug Tracking System Contact ow...@bugs.debian.org with problems -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#773041: Bug#773318: clamav dies/hangs
Control: found 773318 0.98.5+dfsg-0+deb7u1 Control: notfound 773318 0.98.5+dfsg-3 # 0.98.5+dfsg-3 uses the system libmspack # see bug #773041 for progress there Hi Sebastian, On 23.12.2014 21:44, Sebastian Andrzej Siewior wrote: 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. Yes, thanks. 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… I'm marking the unstable version as not affected, because we have #773041 to track the progress for libmspack. So the upload of the fixed wheezy version will close this bug. Side question: In case someone has unattended-upgrades running, how does he get clamd restarted after a libmspack update? I think needrestart can do that. Best regards, Andreas -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Processed: Re: Bug#773041: Bug#773318: clamav dies/hangs
Processing control commands: found 773318 0.98.5+dfsg-0+deb7u1 Bug #773318 [clamav-daemon] clamav dies/hangs Marked as found in versions clamav/0.98.5+dfsg-0+deb7u1. notfound 773318 0.98.5+dfsg-3 Bug #773318 [clamav-daemon] clamav dies/hangs No longer marked as found in versions clamav/0.98.5+dfsg-3. -- 773041: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=773041 773318: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=773318 Debian Bug Tracking System Contact ow...@bugs.debian.org with problems -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Processed: Re: Bug#773041: Bug#773318: clamav dies/hangs
Processing control commands: found 773318 0.98.5+dfsg-0+deb7u1 Bug #773318 [clamav-daemon] clamav dies/hangs Ignoring request to alter found versions of bug #773318 to the same values previously set notfound 773318 0.98.5+dfsg-3 Bug #773318 [clamav-daemon] clamav dies/hangs Ignoring request to alter found versions of bug #773318 to the same values previously set -- 773318: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=773318 Debian Bug Tracking System Contact ow...@bugs.debian.org with problems -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#773041: Bug#773318: clamav dies/hangs
* Marc Dequènes (duck) | 2014-12-22 02:52:02 [+0100]: Coin, On 2014-12-21 22:16, Sebastian Andrzej Siewior wrote: On 2014-12-20 12:12:13 [+0100], Andreas Cadhalpun wrote: As it shows that clamd hangs in libmspack, I think this is bug #773041 [1]. A possible fix is mentioned in [2]. I can upload this simple fix quickly, nevertheless i did not have time to proofread it. Any comment? It would be nice if we could keep this in sync. I will look at this in tonight at the latest and give more feedback. Is the security team aware of the various in-tree copy of this library? #67 tries / tried to track them. Joss filled #675560 tagged security. Yes. Atleast clamav can be triggered via remote. Not sure about the others. Regards. Sebastian -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#773041: Bug#773318: clamav dies/hangs
On 2014-12-22 02:52:02 [+0100], Marc Dequènes (duck) wrote:
I can upload this simple fix quickly, nevertheless i did not have time to
proofread it. Any comment?
I plan to add the following patch to clamav. I added a small comment
why we have the busy loop there. So far it looks like a good idea. The
only problem is that we need off_t beeing 64bit (LFS) or it won't work
on 32bit. No problem on Debian side…
I added upstream on CC hoping that they will take this or do something
about it :)
If nobody objects, I push this tomorrow into the clamav repo.
From 9041fefc0d48aa3c307baa20c5cc4b7eceafe616 Mon Sep 17 00:00:00 2001
From: Sebastian Andrzej Siewior sebast...@breakpoint.cc
Date: Mon, 22 Dec 2014 22:10:47 +0100
Subject: [PATCH] make frame_end off_t
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. He also added that making
it off_t (and so 64bit with LFS) fixes the problem.
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.
This patch is based on Eric Sharkey comments.
Signed-off-by: Sebastian Andrzej Siewior sebast...@breakpoint.cc
---
mspack/qtmd.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/mspack/qtmd.c b/mspack/qtmd.c
index 12b27f5608c4..6e1640579119 100644
--- a/mspack/qtmd.c
+++ b/mspack/qtmd.c
@@ -253,7 +253,8 @@ struct qtmd_stream *qtmd_init(struct mspack_system *system,
}
int qtmd_decompress(struct qtmd_stream *qtm, off_t out_bytes) {
- unsigned int frame_todo, frame_end, window_posn, match_offset, range;
+ unsigned int frame_todo, window_posn, match_offset, range;
+ off_t frame_end;
unsigned char *window, *i_ptr, *i_end, *runsrc, *rundest;
int i, j, selector, extra, sym, match_length;
unsigned short H, L, C, symf;
--
2.1.3
Sebastian
--
To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#773041: Bug#773318: clamav dies/hangs
On 2014-12-20 12:12:13 [+0100], Andreas Cadhalpun wrote: As it shows that clamd hangs in libmspack, I think this is bug #773041 [1]. A possible fix is mentioned in [2]. We'll have to include it in the libmspack copy embedded in clamav, which is used in wheezy. Oh great. So for clamav we have to fix it stable since sid uses the external library (jay!). This type fix does not look that bad. Do we wait for the upstream maintainer on this atm? Is the security team aware of the various in-tree copy of this library? #67 tries / tried to track them. Best regards, Andreas Sebastian -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#773041: Bug#773318: clamav dies/hangs
Coin, On 2014-12-21 22:16, Sebastian Andrzej Siewior wrote: On 2014-12-20 12:12:13 [+0100], Andreas Cadhalpun wrote: As it shows that clamd hangs in libmspack, I think this is bug #773041 [1]. A possible fix is mentioned in [2]. I can upload this simple fix quickly, nevertheless i did not have time to proofread it. Any comment? Is the security team aware of the various in-tree copy of this library? #67 tries / tried to track them. Joss filled #675560 tagged security. I tested the cabextract build using the library but had no reply. I wanted to have at least one real user for the lib before pinging the other related packages. I should probably have been more aggressive on this front, my bad. Regards. -- Marc Dequènes -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
