From 234be0245324b01676aff764b756248f4e57b45d Mon Sep 17 00:00:00 2001 From: Ulrich Sibiller Date: Wed, 19 Jun 2019 23:10:40 +0200 Subject: glyph.c: fix a read beyond end of heap buffer If compiled with -fsanitize=address this showed up when running startlxde: ==11551==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60d000018fbc at pc 0x7f270a9ed57b bp 0x7fff30ef3050 sp 0x7fff30ef2800 READ of size 204 at 0x60d000018fbc thread T0 #0 0x7f270a9ed57a (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xb857a) #1 0x559dafcd5c93 in FindGlyphRef ../../render/glyph.c:179 #2 0x559dafcd705d in AddGlyph /work/nx-libs/nx-X11/programs/Xserver/hw/nxagent/NXglyph.c:71 #3 0x559dafccc0ff in ProcRenderAddGlyphs ../../mi/../render/render.c:1186 #4 0x559dafcbd5a5 in ProcRenderDispatch /work/nx-libs/nx-X11/programs/Xserver/hw/nxagent/NXrender.c:1689 #5 0x559dafcbc4ea in Dispatch /work/nx-libs/nx-X11/programs/Xserver/hw/nxagent/NXdispatch.c:476 #6 0x559dafc4e9b0 in main /work/nx-libs/nx-X11/programs/Xserver/dix/main.c:353 #7 0x7f2708e1d09a in __libc_start_main ../csu/libc-start.c:308 #8 0x559dafc4f5d9 in _start (/work/nx-libs/nx-X11/programs/Xserver/nxagent+0x6e5d9) 0x60d000018fbc is located 0 bytes to the right of 140-byte region [0x60d000018f30,0x60d000018fbc) allocated by thread T0 here: #0 0x7f270aa1e330 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xe9330) #1 0x559dafcd646c in AllocateGlyph ../../render/glyph.c:348 This happens when two glyphs are compared via memcmp and the smaller one happens to be identical to the beginning of the bigger one. Newer render implementations use a sha1 hash instead of memcmp so this patch will (hopefully) be obsolete once render gets updated. --- nx-X11/programs/Xserver/hw/nxagent/NXglyph.c | 61 ++++++++++++++++++++++++++++ nx-X11/programs/Xserver/render/glyph.c | 2 + 2 files changed, 63 insertions(+) diff --git a/nx-X11/programs/Xserver/hw/nxagent/NXglyph.c b/nx-X11/programs/Xserver/hw/nxagent/NXglyph.c index 1f82e73dc..72d8242bd 100644 --- a/nx-X11/programs/Xserver/hw/nxagent/NXglyph.c +++ b/nx-X11/programs/Xserver/hw/nxagent/NXglyph.c @@ -59,6 +59,67 @@ #endif +GlyphRefPtr +FindGlyphRef (GlyphHashPtr hash, CARD32 signature, Bool match, GlyphPtr compare) +{ + CARD32 elt, step, s; + GlyphPtr glyph; + GlyphRefPtr table, gr, del; + CARD32 tableSize = hash->hashSet->size; + + table = hash->table; + elt = signature % tableSize; + step = 0; + del = 0; + for (;;) + { + gr = &table[elt]; + s = gr->signature; + glyph = gr->glyph; + if (!glyph) + { + if (del) + gr = del; + break; + } + if (glyph == DeletedGlyph) + { + if (!del) + del = gr; + else if (gr == del) + break; + } +#ifdef NXAGENT_SERVER + else if (s == signature && match && glyph->size != compare->size) + { + /* + * if the glyphsize is different there's no need to do a memcmp + * because it will surely report difference. And even worse: + * it will read beyond the end of glyph under some + * circumstances, which can be detected when compiling with + * -fsanitize=address. + */ + } +#endif + else if (s == signature && + (!match || + memcmp (&compare->info, &glyph->info, compare->size) == 0)) + { + break; + } + if (!step) + { + step = signature % hash->hashSet->rehash; + if (!step) + step = 1; + } + elt += step; + if (elt >= tableSize) + elt -= tableSize; + } + return gr; +} + void AddGlyph (GlyphSetPtr glyphSet, GlyphPtr glyph, Glyph id) { diff --git a/nx-X11/programs/Xserver/render/glyph.c b/nx-X11/programs/Xserver/render/glyph.c index a379b505f..93aed401b 100644 --- a/nx-X11/programs/Xserver/render/glyph.c +++ b/nx-X11/programs/Xserver/render/glyph.c @@ -144,6 +144,7 @@ GlyphInit (ScreenPtr pScreen) return TRUE; } +#ifndef NXAGENT_SERVER GlyphRefPtr FindGlyphRef (GlyphHashPtr hash, CARD32 signature, Bool match, GlyphPtr compare) { @@ -192,6 +193,7 @@ FindGlyphRef (GlyphHashPtr hash, CARD32 signature, Bool match, GlyphPtr compare) } return gr; } +#endif CARD32 HashGlyph (GlyphPtr glyph) -- cgit v1.2.3