1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
|
From: Tobias Stoeckmann <tobias@stoeckmann.org>
Date: Sat, 21 Oct 2017 23:47:52 +0200
Subject: Fix heap overflows when parsing malicious files. (CVE-2017-16612)
Origin: https://cgit.freedesktop.org/xorg/lib/libXcursor/commit?id=4794b5dd34688158fb51a2943032569d3780c4b8
Bug-Debian: https://bugs.debian.org/883792
Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2017-16612
It is possible to trigger heap overflows due to an integer overflow
while parsing images and a signedness issue while parsing comments.
The integer overflow occurs because the chosen limit 0x10000 for
dimensions is too large for 32 bit systems, because each pixel takes
4 bytes. Properly chosen values allow an overflow which in turn will
lead to less allocated memory than needed for subsequent reads.
The signedness bug is triggered by reading the length of a comment
as unsigned int, but casting it to int when calling the function
XcursorCommentCreate. Turning length into a negative value allows the
check against XCURSOR_COMMENT_MAX_LEN to pass, and the following
addition of sizeof (XcursorComment) + 1 makes it possible to allocate
less memory than needed for subsequent reads.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Reviewed-by: Matthieu Herrb <matthieu@herrb.eu>
---
src/file.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/src/file.c b/src/file.c
index 43163c2..da16277 100644
--- a/src/file.c
+++ b/src/file.c
@@ -29,6 +29,11 @@ XcursorImageCreate (int width, int height)
{
XcursorImage *image;
+ if (width < 0 || height < 0)
+ return NULL;
+ if (width > XCURSOR_IMAGE_MAX_SIZE || height > XCURSOR_IMAGE_MAX_SIZE)
+ return NULL;
+
image = malloc (sizeof (XcursorImage) +
width * height * sizeof (XcursorPixel));
if (!image)
@@ -101,7 +106,7 @@ XcursorCommentCreate (XcursorUInt comment_type, int length)
{
XcursorComment *comment;
- if (length > XCURSOR_COMMENT_MAX_LEN)
+ if (length < 0 || length > XCURSOR_COMMENT_MAX_LEN)
return NULL;
comment = malloc (sizeof (XcursorComment) + length + 1);
@@ -448,7 +453,8 @@ _XcursorReadImage (XcursorFile *file,
if (!_XcursorReadUInt (file, &head.delay))
return NULL;
/* sanity check data */
- if (head.width >= 0x10000 || head.height > 0x10000)
+ if (head.width > XCURSOR_IMAGE_MAX_SIZE ||
+ head.height > XCURSOR_IMAGE_MAX_SIZE)
return NULL;
if (head.width == 0 || head.height == 0)
return NULL;
@@ -457,6 +463,8 @@ _XcursorReadImage (XcursorFile *file,
/* Create the image and initialize it */
image = XcursorImageCreate (head.width, head.height);
+ if (image == NULL)
+ return NULL;
if (chunkHeader.version < image->version)
image->version = chunkHeader.version;
image->size = chunkHeader.subtype;
--
2.15.1
|
