From a483b5c7724469309e3df427730cbb8b805b9c9f Mon Sep 17 00:00:00 2001

From: Peter Hutterer <peter.hutterer@who-t.net>

Date: Thu, 4 Jan 2024 10:01:24 +1000

Subject: [PATCH xserver] Xi: flush hierarchy events after adding/removing

 master devices

The `XISendDeviceHierarchyEvent()` function allocates space to store up

to `MAXDEVICES` (256) `xXIHierarchyInfo` structures in `info`.

If a device with a given ID was removed and a new device with the same

ID added both in the same operation, the single device ID will lead to

two info structures being written to `info`.

Since this case can occur for every device ID at once, a total of two

times `MAXDEVICES` info structures might be written to the allocation.

To avoid it, once one add/remove master is processed, send out the

device hierarchy event for the current state and continue. That event

thus only ever has exactly one of either added/removed in it (and

optionally slave attached/detached).

CVE-2024-21885, ZDI-CAN-22744

This vulnerability was discovered by:

Jan-Niklas Sohn working with Trend Micro Zero Day Initiative

---

 Xi/xichangehierarchy.c | 30 ++++++++++++++++++++++++------

 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/Xi/xichangehierarchy.c b/Xi/xichangehierarchy.c

index 01eb7a8af4..67eedddec6 100644

--- a/Xi/xichangehierarchy.c

+++ b/Xi/xichangehierarchy.c

@@ -340,6 +340,11 @@ ProcXIChangeHierarchy(ClientPtr client)

     size_t len;			/* length of data remaining in request */

     int rc = Success;

     int flags[MAXDEVICES] = { 0 };

+    enum {

+        NO_CHANGE,

+        FLUSH,

+        CHANGED,

+    } changes = NO_CHANGE;

     REQUEST(xXIChangeHierarchyReq);

     REQUEST_AT_LEAST_SIZE(xXIChangeHierarchyReq);

@@ -389,8 +394,9 @@ ProcXIChangeHierarchy(ClientPtr client)

             rc = add_master(client, c, flags);

             if (rc != Success)

                 goto unwind;

-        }

+            changes = FLUSH;

             break;

+        }

         case XIRemoveMaster:

         {

             xXIRemoveMasterInfo *r = (xXIRemoveMasterInfo *) any;

@@ -399,8 +405,9 @@ ProcXIChangeHierarchy(ClientPtr client)

             rc = remove_master(client, r, flags);

             if (rc != Success)

                 goto unwind;

-        }

+            changes = FLUSH;

             break;

+        }

         case XIDetachSlave:

         {

             xXIDetachSlaveInfo *c = (xXIDetachSlaveInfo *) any;

@@ -409,8 +416,9 @@ ProcXIChangeHierarchy(ClientPtr client)

             rc = detach_slave(client, c, flags);

             if (rc != Success)

                 goto unwind;

-        }

+            changes = CHANGED;

             break;

+        }

         case XIAttachSlave:

         {

             xXIAttachSlaveInfo *c = (xXIAttachSlaveInfo *) any;

@@ -495,16 +503,25 @@ ProcXIChangeHierarchy(ClientPtr client)

             rc = attach_slave(client, c, flags);

             if (rc != Success)

                 goto unwind;

+            changes = CHANGED;

+            break;

         }

+        default:

             break;

         }

+        if (changes == FLUSH) {

+            XISendDeviceHierarchyEvent(flags);

+            memset(flags, 0, sizeof(flags));

+            changes = NO_CHANGE;

+        }

+

         len -= any->length * 4;

         any = (xXIAnyHierarchyChangeInfo *) ((char *) any + any->length * 4);

     }

  unwind:

-

-    XISendDeviceHierarchyEvent(flags);

+    if (changes != NO_CHANGE)

+        XISendDeviceHierarchyEvent(flags);

     return rc;

 }

-- 

2.43.0