Skip to content

Commit

Permalink
glyph.c: fix a read beyond end of heap buffer
Browse files Browse the repository at this point in the history
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
    ArcticaProject#4 0x559dafcbd5a5 in ProcRenderDispatch /work/nx-libs/nx-X11/programs/Xserver/hw/nxagent/NXrender.c:1689
    ArcticaProject#5 0x559dafcbc4ea in Dispatch /work/nx-libs/nx-X11/programs/Xserver/hw/nxagent/NXdispatch.c:476
    ArcticaProject#6 0x559dafc4e9b0 in main /work/nx-libs/nx-X11/programs/Xserver/dix/main.c:353
    ArcticaProject#7 0x7f2708e1d09a in __libc_start_main ../csu/libc-start.c:308
    ArcticaProject#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.
  • Loading branch information
uli42 committed Jun 21, 2019
1 parent 187bf87 commit 53d4b74
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 0 deletions.
61 changes: 61 additions & 0 deletions nx-X11/programs/Xserver/hw/nxagent/NXglyph.c
Expand Up @@ -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 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)
{
Expand Down
2 changes: 2 additions & 0 deletions nx-X11/programs/Xserver/render/glyph.c
Expand Up @@ -144,6 +144,7 @@ GlyphInit (ScreenPtr pScreen)
return TRUE;
}

#ifndef NXAGENT_SERVER
GlyphRefPtr
FindGlyphRef (GlyphHashPtr hash, CARD32 signature, Bool match, GlyphPtr compare)
{
Expand Down Expand Up @@ -192,6 +193,7 @@ FindGlyphRef (GlyphHashPtr hash, CARD32 signature, Bool match, GlyphPtr compare)
}
return gr;
}
#endif

CARD32
HashGlyph (GlyphPtr glyph)
Expand Down

0 comments on commit 53d4b74

Please sign in to comment.