forked from pool/xorg-x11-server
76 lines
2.7 KiB
Diff
76 lines
2.7 KiB
Diff
|
From a4d9bf1259ad28f54b6d59a480b2009cc89ca623 Mon Sep 17 00:00:00 2001
|
||
|
From: Alan Coopersmith <alan.coopersmith@oracle.com>
|
||
|
Date: Mon, 16 Sep 2013 21:47:16 -0700
|
||
|
Subject: [PATCH] Avoid use-after-free in dix/dixfonts.c: doImageText()
|
||
|
|
||
|
Save a pointer to the passed in closure structure before copying it
|
||
|
and overwriting the *c pointer to point to our copy instead of the
|
||
|
original. If we hit an error, once we free(c), reset c to point to
|
||
|
the original structure before jumping to the cleanup code that
|
||
|
references *c.
|
||
|
|
||
|
Since one of the errors being checked for is whether the server was
|
||
|
able to malloc(c->nChars * itemSize), the client can potentially pass
|
||
|
a number of characters chosen to cause the malloc to fail and the
|
||
|
error path to be taken, resulting in the read from freed memory.
|
||
|
|
||
|
Since the memory is accessed almost immediately afterwards, and the
|
||
|
X server is mostly single threaded, the odds of the free memory having
|
||
|
invalid contents are low with most malloc implementations when not using
|
||
|
memory debugging features, but some allocators will definitely overwrite
|
||
|
the memory there, leading to a likely crash.
|
||
|
|
||
|
Reported-by: Pedro Ribeiro <pedrib@gmail.com>
|
||
|
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
|
||
|
Reviewed-by: Julien Cristau <jcristau@debian.org>
|
||
|
---
|
||
|
dix/dixfonts.c | 5 +++++
|
||
|
1 file changed, 5 insertions(+)
|
||
|
|
||
|
diff --git a/dix/dixfonts.c b/dix/dixfonts.c
|
||
|
index feb765d..2e34d37 100644
|
||
|
--- a/dix/dixfonts.c
|
||
|
+++ b/dix/dixfonts.c
|
||
|
@@ -1425,6 +1425,7 @@ doImageText(ClientPtr client, ITclosurePtr c)
|
||
|
GC *pGC;
|
||
|
unsigned char *data;
|
||
|
ITclosurePtr new_closure;
|
||
|
+ ITclosurePtr old_closure;
|
||
|
|
||
|
/* We're putting the client to sleep. We need to
|
||
|
save some state. Similar problem to that handled
|
||
|
@@ -1436,12 +1437,14 @@ doImageText(ClientPtr client, ITclosurePtr c)
|
||
|
err = BadAlloc;
|
||
|
goto bail;
|
||
|
}
|
||
|
+ old_closure = c;
|
||
|
*new_closure = *c;
|
||
|
c = new_closure;
|
||
|
|
||
|
data = malloc(c->nChars * itemSize);
|
||
|
if (!data) {
|
||
|
free(c);
|
||
|
+ c = old_closure;
|
||
|
err = BadAlloc;
|
||
|
goto bail;
|
||
|
}
|
||
|
@@ -1452,6 +1455,7 @@ doImageText(ClientPtr client, ITclosurePtr c)
|
||
|
if (!pGC) {
|
||
|
free(c->data);
|
||
|
free(c);
|
||
|
+ c = old_closure;
|
||
|
err = BadAlloc;
|
||
|
goto bail;
|
||
|
}
|
||
|
@@ -1464,6 +1468,7 @@ doImageText(ClientPtr client, ITclosurePtr c)
|
||
|
FreeScratchGC(pGC);
|
||
|
free(c->data);
|
||
|
free(c);
|
||
|
+ c = old_closure;
|
||
|
err = BadAlloc;
|
||
|
goto bail;
|
||
|
}
|
||
|
--
|
||
|
1.7.9.2
|
||
|
|