mirror/mpv
0
0
Fork 0
mirror of https://github.com/mpv-player/mpv.git synced 2024-09-20 03:52:22 +02:00
Code

x11: fix X property out of bounds memory reads

Browse Source 
The size overflow check was inverted: instead of allowing reading only
the first dst_size bytes of the property, it allowed copying past the
property buffer (as returned by xlib). xlib doesn't return the size of
the buffer in bytes, so it has to be computed and checked manually.

Wouldn't it be great if C allowed me to write the overflow check in a
readable way, so it doesn't trick me into writing dumb security bugs?

Relying on X security is even dumber than creating a X security bug,
though, so this was not a real problem. But I found that one specific
call tried to read more than what the property provided, so reduce that.

Also, len*ib obviously can't overflow, so there's an additional layer of
dumb to this whole thing.

While we're at dumb things, why the hell does xlib use "long" for 32 bit
types. It's a god damn pain.
This commit is contained in:
wm4 2019-12-18 07:12:53 +01:00
parent 5f74ed5828
commit 11d35b72a6
1 changed files with 2 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
video/out/x11_common.c
View File

@ -207,7 +207,7 @@ static bool x11_get_property_copy(struct vo_x11_state *x11, Window w,
void *ptr = x11_get_property(x11, w, property, type, format, &len);
if (ptr) {
size_t ib = format == 32 ? sizeof(long) : format / 8;
if (dst_size / ib >= len) {
if (dst_size <= len * ib) {
memcpy(dst, ptr, dst_size);
ret = true;
}
@ -1079,7 +1079,7 @@ static void vo_x11_check_net_wm_desktop_change(struct vo *vo)
if (x11->parent)
return;
long params[5] = {0};
long params[1] = {0};
if (x11_get_property_copy(x11, x11->window, XA(x11, _NET_WM_DESKTOP),
XA_CARDINAL, 32, params, sizeof(params)))
{