Peter Hutterer 75e6f92
From a42635ee3c01f71a49052d83a372933504c9db04 Mon Sep 17 00:00:00 2001
Peter Hutterer 75e6f92
From: Peter Hutterer <peter.hutterer@who-t.net>
Peter Hutterer 75e6f92
Date: Wed, 30 Nov 2022 11:20:40 +1000
Peter Hutterer 75e6f92
Subject: [PATCH xserver 6/7] Xext: free the XvRTVideoNotify when turning off
Peter Hutterer 75e6f92
 from the same client
Peter Hutterer 75e6f92
Peter Hutterer 75e6f92
This fixes a use-after-free bug:
Peter Hutterer 75e6f92
Peter Hutterer 75e6f92
When a client first calls XvdiSelectVideoNotify() on a drawable with a
Peter Hutterer 75e6f92
TRUE onoff argument, a struct XvVideoNotifyRec is allocated. This struct
Peter Hutterer 75e6f92
is added twice to the resources:
Peter Hutterer 75e6f92
  - as the drawable's XvRTVideoNotifyList. This happens only once per
Peter Hutterer 75e6f92
    drawable, subsequent calls append to this list.
Peter Hutterer 75e6f92
  - as the client's XvRTVideoNotify. This happens for every client.
Peter Hutterer 75e6f92
Peter Hutterer 75e6f92
The struct keeps the ClientPtr around once it has been added for a
Peter Hutterer 75e6f92
client. The idea, presumably, is that if the client disconnects we can remove
Peter Hutterer 75e6f92
all structs from the drawable's list that match the client (by resetting
Peter Hutterer 75e6f92
the ClientPtr to NULL), but if the drawable is destroyed we can remove
Peter Hutterer 75e6f92
and free the whole list.
Peter Hutterer 75e6f92
Peter Hutterer 75e6f92
However, if the same client then calls XvdiSelectVideoNotify() on the
Peter Hutterer 75e6f92
same drawable with a FALSE onoff argument, only the ClientPtr on the
Peter Hutterer 75e6f92
existing struct was set to NULL. The struct itself remained in the
Peter Hutterer 75e6f92
client's resources.
Peter Hutterer 75e6f92
Peter Hutterer 75e6f92
If the drawable is now destroyed, the resource system invokes
Peter Hutterer 75e6f92
XvdiDestroyVideoNotifyList which frees the whole list for this drawable
Peter Hutterer 75e6f92
- including our struct. This function however does not free the resource
Peter Hutterer 75e6f92
for the client since our ClientPtr is NULL.
Peter Hutterer 75e6f92
Peter Hutterer 75e6f92
Later, when the client is destroyed and the resource system invokes
Peter Hutterer 75e6f92
XvdiDestroyVideoNotify, we unconditionally set the ClientPtr to NULL. On
Peter Hutterer 75e6f92
a struct that has been freed previously. This is generally frowned upon.
Peter Hutterer 75e6f92
Peter Hutterer 75e6f92
Fix this by calling FreeResource() on the second call instead of merely
Peter Hutterer 75e6f92
setting the ClientPtr to NULL. This removes the struct from the client
Peter Hutterer 75e6f92
resources (but not from the list), ensuring that it won't be accessed
Peter Hutterer 75e6f92
again when the client quits.
Peter Hutterer 75e6f92
Peter Hutterer 75e6f92
Note that the assignment tpn->client = NULL; is superfluous since the
Peter Hutterer 75e6f92
XvdiDestroyVideoNotify function will do this anyway. But it's left for
Peter Hutterer 75e6f92
clarity and to match a similar invocation in XvdiSelectPortNotify.
Peter Hutterer 75e6f92
Peter Hutterer 75e6f92
CVE-2022-46342, ZDI-CAN 19400
Peter Hutterer 75e6f92
Peter Hutterer 75e6f92
This vulnerability was discovered by:
Peter Hutterer 75e6f92
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
Peter Hutterer 75e6f92
Peter Hutterer 75e6f92
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Peter Hutterer 75e6f92
Acked-by: Olivier Fourdan <ofourdan@redhat.com>
Peter Hutterer 75e6f92
---
Peter Hutterer 75e6f92
 Xext/xvmain.c | 4 +++-
Peter Hutterer 75e6f92
 1 file changed, 3 insertions(+), 1 deletion(-)
Peter Hutterer 75e6f92
Peter Hutterer 75e6f92
diff --git a/Xext/xvmain.c b/Xext/xvmain.c
Peter Hutterer 75e6f92
index f627471938..2a08f8744a 100644
Peter Hutterer 75e6f92
--- a/Xext/xvmain.c
Peter Hutterer 75e6f92
+++ b/Xext/xvmain.c
Peter Hutterer 75e6f92
@@ -811,8 +811,10 @@ XvdiSelectVideoNotify(ClientPtr client, DrawablePtr pDraw, BOOL onoff)
Peter Hutterer 75e6f92
         tpn = pn;
Peter Hutterer 75e6f92
         while (tpn) {
Peter Hutterer 75e6f92
             if (tpn->client == client) {
Peter Hutterer 75e6f92
-                if (!onoff)
Peter Hutterer 75e6f92
+                if (!onoff) {
Peter Hutterer 75e6f92
                     tpn->client = NULL;
Peter Hutterer 75e6f92
+                    FreeResource(tpn->id, XvRTVideoNotify);
Peter Hutterer 75e6f92
+                }
Peter Hutterer 75e6f92
                 return Success;
Peter Hutterer 75e6f92
             }
Peter Hutterer 75e6f92
             if (!tpn->client)
Peter Hutterer 75e6f92
-- 
Peter Hutterer 75e6f92
2.38.1
Peter Hutterer 75e6f92