Commit | Line | Data |
---|---|---|
e73631a9 LF |
1 | Fix CVE-2016-5407: |
2 | ||
3 | https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2016-5407 | |
4 | ||
5 | Patch copied from upstream source repository: | |
6 | ||
7 | https://cgit.freedesktop.org/xorg/lib/libXv/commit/?id=d9da580b46a28ab497de2e94fdc7b9ff953dab17 | |
8 | ||
9 | From d9da580b46a28ab497de2e94fdc7b9ff953dab17 Mon Sep 17 00:00:00 2001 | |
10 | From: Tobias Stoeckmann <tobias@stoeckmann.org> | |
11 | Date: Sun, 25 Sep 2016 21:30:03 +0200 | |
12 | Subject: [PATCH] Protocol handling issues in libXv - CVE-2016-5407 | |
13 | ||
14 | The Xv query functions for adaptors and encodings suffer from out of | |
15 | boundary accesses if a hostile X server sends a maliciously crafted | |
16 | response. | |
17 | ||
18 | A previous fix already checks the received length against fixed values | |
19 | but ignores additional length specifications which are stored inside | |
20 | the received data. | |
21 | ||
22 | These lengths are accessed in a for-loop. The easiest way to guarantee | |
23 | a correct processing is by validating all lengths against the | |
24 | remaining size left before accessing referenced memory. | |
25 | ||
26 | This makes the previously applied check obsolete, therefore I removed | |
27 | it. | |
28 | ||
29 | Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org> | |
30 | Reviewed-by: Matthieu Herrb <matthieu@herrb.eu> | |
31 | --- | |
32 | src/Xv.c | 46 +++++++++++++++++++++++++++++----------------- | |
33 | 1 file changed, 29 insertions(+), 17 deletions(-) | |
34 | ||
35 | diff --git a/src/Xv.c b/src/Xv.c | |
36 | index e47093a..be450c4 100644 | |
37 | --- a/src/Xv.c | |
38 | +++ b/src/Xv.c | |
39 | @@ -158,6 +158,7 @@ XvQueryAdaptors( | |
40 | size_t size; | |
41 | unsigned int ii, jj; | |
42 | char *name; | |
43 | + char *end; | |
44 | XvAdaptorInfo *pas = NULL, *pa; | |
45 | XvFormat *pfs, *pf; | |
46 | char *buffer = NULL; | |
47 | @@ -197,17 +198,13 @@ XvQueryAdaptors( | |
48 | /* GET INPUT ADAPTORS */ | |
49 | ||
50 | if (rep.num_adaptors == 0) { | |
51 | - /* If there's no adaptors, there's nothing more to do. */ | |
52 | + /* If there are no adaptors, there's nothing more to do. */ | |
53 | status = Success; | |
54 | goto out; | |
55 | } | |
56 | ||
57 | - if (size < (rep.num_adaptors * sz_xvAdaptorInfo)) { | |
58 | - /* If there's not enough data for the number of adaptors, | |
59 | - then we have a problem. */ | |
60 | - status = XvBadReply; | |
61 | - goto out; | |
62 | - } | |
63 | + u.buffer = buffer; | |
64 | + end = buffer + size; | |
65 | ||
66 | size = rep.num_adaptors * sizeof(XvAdaptorInfo); | |
67 | if ((pas = Xmalloc(size)) == NULL) { | |
68 | @@ -225,9 +222,12 @@ XvQueryAdaptors( | |
69 | pa++; | |
70 | } | |
71 | ||
72 | - u.buffer = buffer; | |
73 | pa = pas; | |
74 | for (ii = 0; ii < rep.num_adaptors; ii++) { | |
75 | + if (u.buffer + sz_xvAdaptorInfo > end) { | |
76 | + status = XvBadReply; | |
77 | + goto out; | |
78 | + } | |
79 | pa->type = u.pa->type; | |
80 | pa->base_id = u.pa->base_id; | |
81 | pa->num_ports = u.pa->num_ports; | |
82 | @@ -239,6 +239,10 @@ XvQueryAdaptors( | |
83 | size = u.pa->name_size; | |
84 | u.buffer += pad_to_int32(sz_xvAdaptorInfo); | |
85 | ||
86 | + if (u.buffer + size > end) { | |
87 | + status = XvBadReply; | |
88 | + goto out; | |
89 | + } | |
90 | if ((name = Xmalloc(size + 1)) == NULL) { | |
91 | status = XvBadAlloc; | |
92 | goto out; | |
93 | @@ -259,6 +263,11 @@ XvQueryAdaptors( | |
94 | ||
95 | pf = pfs; | |
96 | for (jj = 0; jj < pa->num_formats; jj++) { | |
97 | + if (u.buffer + sz_xvFormat > end) { | |
98 | + Xfree(pfs); | |
99 | + status = XvBadReply; | |
100 | + goto out; | |
101 | + } | |
102 | pf->depth = u.pf->depth; | |
103 | pf->visual_id = u.pf->visual; | |
104 | pf++; | |
105 | @@ -327,6 +336,7 @@ XvQueryEncodings( | |
106 | size_t size; | |
107 | unsigned int jj; | |
108 | char *name; | |
109 | + char *end; | |
110 | XvEncodingInfo *pes = NULL, *pe; | |
111 | char *buffer = NULL; | |
112 | union { | |
113 | @@ -364,17 +374,13 @@ XvQueryEncodings( | |
114 | /* GET ENCODINGS */ | |
115 | ||
116 | if (rep.num_encodings == 0) { | |
117 | - /* If there's no encodings, there's nothing more to do. */ | |
118 | + /* If there are no encodings, there's nothing more to do. */ | |
119 | status = Success; | |
120 | goto out; | |
121 | } | |
122 | ||
123 | - if (size < (rep.num_encodings * sz_xvEncodingInfo)) { | |
124 | - /* If there's not enough data for the number of adaptors, | |
125 | - then we have a problem. */ | |
126 | - status = XvBadReply; | |
127 | - goto out; | |
128 | - } | |
129 | + u.buffer = buffer; | |
130 | + end = buffer + size; | |
131 | ||
132 | size = rep.num_encodings * sizeof(XvEncodingInfo); | |
133 | if ((pes = Xmalloc(size)) == NULL) { | |
134 | @@ -391,10 +397,12 @@ XvQueryEncodings( | |
135 | pe++; | |
136 | } | |
137 | ||
138 | - u.buffer = buffer; | |
139 | - | |
140 | pe = pes; | |
141 | for (jj = 0; jj < rep.num_encodings; jj++) { | |
142 | + if (u.buffer + sz_xvEncodingInfo > end) { | |
143 | + status = XvBadReply; | |
144 | + goto out; | |
145 | + } | |
146 | pe->encoding_id = u.pe->encoding; | |
147 | pe->width = u.pe->width; | |
148 | pe->height = u.pe->height; | |
149 | @@ -405,6 +413,10 @@ XvQueryEncodings( | |
150 | size = u.pe->name_size; | |
151 | u.buffer += pad_to_int32(sz_xvEncodingInfo); | |
152 | ||
153 | + if (u.buffer + size > end) { | |
154 | + status = XvBadReply; | |
155 | + goto out; | |
156 | + } | |
157 | if ((name = Xmalloc(size + 1)) == NULL) { | |
158 | status = XvBadAlloc; | |
159 | goto out; | |
160 | -- | |
161 | 2.10.1 | |
162 |