summaryrefslogtreecommitdiffstats
path: root/video
diff options
context:
space:
mode:
authorwm4 <wm4@nowhere>2019-12-18 07:12:53 +0100
committerwm4 <wm4@nowhere>2019-12-18 07:12:53 +0100
commit11d35b72a6fdd58f59ea7e593a4a5267f54b2e54 (patch)
treeb38758cebddaa3cdfca83ec0446239110572b33d /video
parent5f74ed58286a1339412554932f31844ec1b64280 (diff)
downloadmpv-11d35b72a6fdd58f59ea7e593a4a5267f54b2e54.tar.bz2
mpv-11d35b72a6fdd58f59ea7e593a4a5267f54b2e54.tar.xz
x11: fix X property out of bounds memory reads
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.
Diffstat (limited to 'video')
-rw-r--r--video/out/x11_common.c4
1 files changed, 2 insertions, 2 deletions
diff --git a/video/out/x11_common.c b/video/out/x11_common.c
index 92dbfc2aa3..8c37074e0b 100644
--- a/video/out/x11_common.c
+++ b/video/out/x11_common.c
@@ -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)))
{