Package: libimlib2
Version: 1.4.6-2+deb8u1
Severity: normal
Tags: upstream patch
Dear Maintainer,
1) I re-compiled imlib2 package with debug information,
2) compiled and installed tests (data, src/bin),
3) run `valgrind imlib2_test`,
4) moved mouse to right lower corner of window;
==16086== Invalid read of size 1
==16086== at 0x4E79C4E: __imlib_MergeUpdate (in
/usr/lib/x86_64-linux-gnu/libImlib2.so.1.4.6)
==16086== by 0x401773: main (in /usr/bin/imlib2_test)
==16086== Address 0x9d20360 is 0 bytes after a block of size 1,200
alloc'd
==16086== at 0x4C28C20: malloc (vg_replace_malloc.c:296)
==16086== by 0x4E798E3: __imlib_MergeUpdate (in
/usr/lib/x86_64-linux-gnu/libImlib2.so.1.4.6)
==16086== by 0x401773: main (in /usr/bin/imlib2_test)
In gdb, it points to src/lib/updates.c:
| for (xx = x + 1, ww = 1; |
>| (T(xx, y).used & T_USED) && (xx < tw); xx++,|
| for (yy = y + 1, hh = 1, ok = 1; |
xx is 20 and tw is 20, so T(xx, y) addresses one byte out of buffer.
Pretty obvious, off-by-one error due to swapped condition order.
In unlucky case, this can result in application crash.
Security implications: very minor, DoS at most, only for application
drawing images using coordinates from untrusted source ("drawing images
from untrusted sources" by itself is safe).
Two *alternative* patches attached (apply only *one* of them).
TODO: I have not tried to search for similar pattern over codebase yet.
Note: there are no changes affecting this code in 1.4.7 (sid) or 1.4.8
(upstream).
-- System Information:
Debian Release: 8.3
APT prefers stable-updates
APT policy: (500, 'stable-updates'), (500, 'stable'), (100,
'proposed-updates')
Architecture: i386 (x86_64)
Foreign Architectures: amd64
Kernel: Linux 3.16.0-4-amd64 (SMP w/2 CPU cores)
Locale: LANG=ja_JP.UTF-8, LC_CTYPE=ja_JP.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)
Versions of packages libimlib2:amd64 depends on:
ii libbz2-1.0 1.0.6-7+b3
ii libc6 2.19-18+deb8u4
ii libfreetype6 2.5.2-3+deb8u1
ii libgif4 4.1.6-11+deb8u1
ii libid3tag0 0.15.1b-11.1~local1
ii libjpeg62-turbo 1:1.3.1-12
ii libpng12-0 1.2.50-2+deb8u2
ii libtiff5 4.0.3-12.3+deb8u1
ii libx11-6 2:1.6.2-3
ii libxext6 2:1.3.3-1
ii multiarch-support 2.19-18+deb8u4
ii zlib1g 1:1.2.8.dfsg-2+b1
libimlib2:amd64 recommends no packages.
libimlib2:amd64 suggests no packages.
-- no debconf information
Description: off-by-one out-of-bound read due to reversed condtion
Author: Yuriy M. Kaminksiy <[email protected]>
Note: you need *either* off-by-one-alternative.patch, *or* this patch;
DO NOT APPLY BOTH! (it won't break, but would unnecessarily clutter code)
Index: imlib2-1.4.6/src/lib/updates.c
===================================================================
--- imlib2-1.4.6.orig/src/lib/updates.c
+++ imlib2-1.4.6/src/lib/updates.c
@@ -111,7 +111,7 @@ __imlib_MergeUpdate(ImlibUpdate * u, int
int xx, yy, ww, hh, ok;
for (xx = x + 1, ww = 1;
- (T(xx, y).used & T_USED) && (xx < tw); xx++, ww++);
+ (xx < tw) && (T(xx, y).used & T_USED); xx++, ww++);
for (yy = y + 1, hh = 1, ok = 1;
(yy < th) && (ok); yy++, hh++)
{
Description: off-by-one out-of-bound read due to reversed condtion, alternative solution (allocates one more guard cell).
Author: Yuriy M. Kaminksiy <[email protected]>
Note: you need *either* off-by-one-reversed-condition.patch, *or* this patch;
DO NOT APPLY BOTH! (it won't break, but would unnecessarily clutter code)
Index: imlib2-1.4.6/src/lib/updates.c
===================================================================
--- imlib2-1.4.6.orig/src/lib/updates.c
+++ imlib2-1.4.6/src/lib/updates.c
@@ -34,13 +34,14 @@ __imlib_MergeUpdate(ImlibUpdate * u, int
th = h >> TB;
if (h & TM)
th++;
- t = malloc(tw * th * sizeof(struct _tile));
+ t = malloc((tw * th + 1) * sizeof(struct _tile));
/* fill in tiles to be all not used */
for (i = 0, y = 0; y < th; y++)
{
for (x = 0; x < tw; x++)
t[i++].used = T_UNUSED;
}
+ t[i].used = T_UNUSED; /* guard OOB */
/* fill in all tiles */
for (uu = u; uu; uu = uu->next)
{
