Script 'mail_helper' called by obssrc
Hello community,

here is the log from the commit of package xorg-x11-server for openSUSE:Factory 
checked in at 2022-12-15 19:24:14
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Comparing /work/SRC/openSUSE:Factory/xorg-x11-server (Old)
 and      /work/SRC/openSUSE:Factory/.xorg-x11-server.new.1835 (New)
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

Package is "xorg-x11-server"

Thu Dec 15 19:24:14 2022 rev:414 rq:1042895 version:21.1.4

Changes:
--------
--- /work/SRC/openSUSE:Factory/xorg-x11-server/xorg-x11-server.changes  
2022-11-08 10:53:55.989592588 +0100
+++ 
/work/SRC/openSUSE:Factory/.xorg-x11-server.new.1835/xorg-x11-server.changes    
    2022-12-15 19:24:17.263743105 +0100
@@ -1,0 +2,29 @@
+Tue Dec  6 14:26:07 UTC 2022 - Stefan Dirsch <sndir...@suse.com>
+
+- U_0007-xkb-reset-the-radio_groups-pointer-to-NULL-after-fre.patch
+  * XkbGetKbdByName use-after-free (ZDI-CAN-19530, CVE-2022-4283,
+    bsc#1206017)
+
+-------------------------------------------------------------------
+Wed Nov 30 14:51:14 UTC 2022 - Stefan Dirsch <sndir...@suse.com>
+
+- U_0001-Xtest-disallow-GenericEvents-in-XTestSwapFakeInput.patch
+  * Server XTestSwapFakeInput stack overflow (ZDI-CAN 19265,
+    CVE-2022-46340, bsc#1205874)
+- U_0002-Xi-return-an-error-from-XI-property-changes-if-verif.patch
+  * Xi: return an error from XI property changes if verification
+    failed (no ZDI-CAN id, no CVE id, bsc#1205875)
+- U_0003-Xi-avoid-integer-truncation-in-length-check-of-ProcX.patch
+  * Server XIChangeProperty out-of-bounds access (ZDI-CAN 19405,
+    CVE-2022-46344, bsc#1205876)
+- U_0004-Xi-disallow-passive-grabs-with-a-detail-255.patch
+  * Server XIPassiveUngrabDevice out-of-bounds access (ZDI-CAN 19381,
+    CVE-2022-46341, bsc#1205877)
+- U_0005-Xext-free-the-screen-saver-resource-when-replacing-i.patch
+  * Server ScreenSaverSetAttributes use-after-free (ZDI-CAN 19404,
+    CVE-2022-46343, bsc#1205878)
+- U_0006-Xext-free-the-XvRTVideoNotify-when-turning-off-from-.patch
+  * Server XvdiSelectVideoNotify use-after-free (ZDI-CAN 19400,
+    CVE-2022-46342, bsc#1205879)
+
+-------------------------------------------------------------------

New:
----
  U_0001-Xtest-disallow-GenericEvents-in-XTestSwapFakeInput.patch
  U_0002-Xi-return-an-error-from-XI-property-changes-if-verif.patch
  U_0003-Xi-avoid-integer-truncation-in-length-check-of-ProcX.patch
  U_0004-Xi-disallow-passive-grabs-with-a-detail-255.patch
  U_0005-Xext-free-the-screen-saver-resource-when-replacing-i.patch
  U_0006-Xext-free-the-XvRTVideoNotify-when-turning-off-from-.patch
  U_0007-xkb-reset-the-radio_groups-pointer-to-NULL-after-fre.patch

++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

Other differences:
------------------
++++++ xorg-x11-server.spec ++++++
--- /var/tmp/diff_new_pack.SWxWbx/_old  2022-12-15 19:24:18.567750525 +0100
+++ /var/tmp/diff_new_pack.SWxWbx/_new  2022-12-15 19:24:18.571750547 +0100
@@ -249,6 +249,14 @@
 Patch1204412:   U_xkb-proof-GetCountedString-against-request-length-at.patch
 Patch1204416:   U_xkb-fix-some-possible-memleaks-in-XkbGetKbdByName.patch
 
+Patch1205874:   U_0001-Xtest-disallow-GenericEvents-in-XTestSwapFakeInput.patch
+Patch1205875:   
U_0002-Xi-return-an-error-from-XI-property-changes-if-verif.patch
+Patch1205876:   
U_0003-Xi-avoid-integer-truncation-in-length-check-of-ProcX.patch
+Patch1205877:   U_0004-Xi-disallow-passive-grabs-with-a-detail-255.patch
+Patch1205878:   
U_0005-Xext-free-the-screen-saver-resource-when-replacing-i.patch
+Patch1205879:   
U_0006-Xext-free-the-XvRTVideoNotify-when-turning-off-from-.patch
+Patch1206017:   
U_0007-xkb-reset-the-radio_groups-pointer-to-NULL-after-fre.patch
+
 %description
 This package contains the X.Org Server.
 
@@ -408,6 +416,13 @@
 %patch1960 -p1
 %patch1204412 -p1
 %patch1204416 -p1
+%patch1205874 -p1
+%patch1205875 -p1
+%patch1205876 -p1
+%patch1205877 -p1
+%patch1205878 -p1
+%patch1205879 -p1
+%patch1206017 -p1
 
 %build
 # We have some -z now related errors during X default startup (boo#1197994):

++++++ U_0001-Xtest-disallow-GenericEvents-in-XTestSwapFakeInput.patch ++++++
>From 2e8916efe9a8566f97a4c2231891ad0f555fced1 Mon Sep 17 00:00:00 2001
From: Peter Hutterer <peter.hutte...@who-t.net>
Date: Tue, 29 Nov 2022 12:55:45 +1000
Subject: [PATCH xserver 1/6] Xtest: disallow GenericEvents in
 XTestSwapFakeInput

XTestSwapFakeInput assumes all events in this request are
sizeof(xEvent) and iterates through these in 32-byte increments.
However, a GenericEvent may be of arbitrary length longer than 32 bytes,
so any GenericEvent in this list would result in subsequent events to be
misparsed.

Additional, the swapped event is written into a stack-allocated struct
xEvent (size 32 bytes). For any GenericEvent longer than 32 bytes,
swapping the event may thus smash the stack like an avocado on toast.

Catch this case early and return BadValue for any GenericEvent.
Which is what would happen in unswapped setups anyway since XTest
doesn't support GenericEvent.

ZDI-CAN 19265

This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative

Signed-off-by: Peter Hutterer <peter.hutte...@who-t.net>
---
 Xext/xtest.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Xext/xtest.c b/Xext/xtest.c
index bf27eb590b..2985a4ce6e 100644
--- a/Xext/xtest.c
+++ b/Xext/xtest.c
@@ -502,10 +502,11 @@ XTestSwapFakeInput(ClientPtr client, xReq * req)
 
     nev = ((req->length << 2) - sizeof(xReq)) / sizeof(xEvent);
     for (ev = (xEvent *) &req[1]; --nev >= 0; ev++) {
+        int evtype = ev->u.u.type & 0x177;
         /* Swap event */
-        proc = EventSwapVector[ev->u.u.type & 0177];
+        proc = EventSwapVector[evtype];
         /* no swapping proc; invalid event type? */
-        if (!proc || proc == NotImplemented) {
+        if (!proc || proc == NotImplemented || evtype == GenericEvent) {
             client->errorValue = ev->u.u.type;
             return BadValue;
         }
-- 
2.38.1


++++++ U_0002-Xi-return-an-error-from-XI-property-changes-if-verif.patch ++++++
>From bee46f23fbc2b2722753c3b7769c990b90c235a0 Mon Sep 17 00:00:00 2001
From: Peter Hutterer <peter.hutte...@who-t.net>
Date: Tue, 29 Nov 2022 13:24:00 +1000
Subject: [PATCH xserver 2/6] Xi: return an error from XI property changes if
 verification failed

Both ProcXChangeDeviceProperty and ProcXIChangeProperty checked the
property for validity but didn't actually return the potential error.

Signed-off-by: Peter Hutterer <peter.hutte...@who-t.net>
---
 Xi/xiproperty.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Xi/xiproperty.c b/Xi/xiproperty.c
index a36f7d61df..68c362c628 100644
--- a/Xi/xiproperty.c
+++ b/Xi/xiproperty.c
@@ -902,6 +902,8 @@ ProcXChangeDeviceProperty(ClientPtr client)
 
     rc = check_change_property(client, stuff->property, stuff->type,
                                stuff->format, stuff->mode, stuff->nUnits);
+    if (rc != Success)
+        return rc;
 
     len = stuff->nUnits;
     if (len > (bytes_to_int32(0xffffffff - sizeof(xChangeDevicePropertyReq))))
@@ -1141,6 +1143,9 @@ ProcXIChangeProperty(ClientPtr client)
 
     rc = check_change_property(client, stuff->property, stuff->type,
                                stuff->format, stuff->mode, stuff->num_items);
+    if (rc != Success)
+        return rc;
+
     len = stuff->num_items;
     if (len > bytes_to_int32(0xffffffff - sizeof(xXIChangePropertyReq)))
         return BadLength;
-- 
2.38.1


++++++ U_0003-Xi-avoid-integer-truncation-in-length-check-of-ProcX.patch ++++++
>From 6f01a643c90724f32c19985e39de3bee9b14a310 Mon Sep 17 00:00:00 2001
From: Peter Hutterer <peter.hutte...@who-t.net>
Date: Tue, 29 Nov 2022 13:26:57 +1000
Subject: [PATCH xserver 3/6] Xi: avoid integer truncation in length check of
 ProcXIChangeProperty

This fixes an OOB read and the resulting information disclosure.

Length calculation for the request was clipped to a 32-bit integer. With
the correct stuff->num_items value the expected request size was
truncated, passing the REQUEST_FIXED_SIZE check.

The server then proceeded with reading at least stuff->num_items bytes
(depending on stuff->format) from the request and stuffing whatever it
finds into the property. In the process it would also allocate at least
stuff->num_items bytes, i.e. 4GB.

The same bug exists in ProcChangeProperty and ProcXChangeDeviceProperty,
so let's fix that too.

ZDI-CAN 19405

This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative

Signed-off-by: Peter Hutterer <peter.hutte...@who-t.net>
---
 Xi/xiproperty.c | 4 ++--
 dix/property.c  | 3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/Xi/xiproperty.c b/Xi/xiproperty.c
index 68c362c628..066ba21fba 100644
--- a/Xi/xiproperty.c
+++ b/Xi/xiproperty.c
@@ -890,7 +890,7 @@ ProcXChangeDeviceProperty(ClientPtr client)
     REQUEST(xChangeDevicePropertyReq);
     DeviceIntPtr dev;
     unsigned long len;
-    int totalSize;
+    uint64_t totalSize;
     int rc;
 
     REQUEST_AT_LEAST_SIZE(xChangeDevicePropertyReq);
@@ -1130,7 +1130,7 @@ ProcXIChangeProperty(ClientPtr client)
 {
     int rc;
     DeviceIntPtr dev;
-    int totalSize;
+    uint64_t totalSize;
     unsigned long len;
 
     REQUEST(xXIChangePropertyReq);
diff --git a/dix/property.c b/dix/property.c
index 94ef5a0ec0..acce94b2c6 100644
--- a/dix/property.c
+++ b/dix/property.c
@@ -205,7 +205,8 @@ ProcChangeProperty(ClientPtr client)
     WindowPtr pWin;
     char format, mode;
     unsigned long len;
-    int sizeInBytes, totalSize, err;
+    int sizeInBytes, err;
+    uint64_t totalSize;
 
     REQUEST(xChangePropertyReq);
 
-- 
2.38.1


++++++ U_0004-Xi-disallow-passive-grabs-with-a-detail-255.patch ++++++
>From 9dc018a5a1a183e0a2cb945572454779b499430c Mon Sep 17 00:00:00 2001
From: Peter Hutterer <peter.hutte...@who-t.net>
Date: Tue, 29 Nov 2022 13:55:32 +1000
Subject: [PATCH xserver 4/6] Xi: disallow passive grabs with a detail > 255

The XKB protocol effectively prevents us from ever using keycodes above
255. For buttons it's theoretically possible but realistically too niche
to worry about. For all other passive grabs, the detail must be zero
anyway.

This fixes an OOB write:

ProcXIPassiveUngrabDevice() calls DeletePassiveGrabFromList with a
temporary grab struct which contains tempGrab->detail.exact = stuff->detail.
For matching existing grabs, DeleteDetailFromMask is called with the
stuff->detail value. This function creates a new mask with the one bit
representing stuff->detail cleared.

However, the array size for the new mask is 8 * sizeof(CARD32) bits,
thus any detail above 255 results in an OOB array write.

ZDI-CAN 19381

This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative

Signed-off-by: Peter Hutterer <peter.hutte...@who-t.net>
---
 Xi/xipassivegrab.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/Xi/xipassivegrab.c b/Xi/xipassivegrab.c
index 2769fb7c94..c9ac2f8553 100644
--- a/Xi/xipassivegrab.c
+++ b/Xi/xipassivegrab.c
@@ -137,6 +137,12 @@ ProcXIPassiveGrabDevice(ClientPtr client)
         return BadValue;
     }
 
+    /* XI2 allows 32-bit keycodes but thanks to XKB we can never
+     * implement this. Just return an error for all keycodes that
+     * cannot work anyway, same for buttons > 255. */
+    if (stuff->detail > 255)
+        return XIAlreadyGrabbed;
+
     if (XICheckInvalidMaskBits(client, (unsigned char *) &stuff[1],
                                stuff->mask_len * 4) != Success)
         return BadValue;
@@ -207,14 +213,8 @@ ProcXIPassiveGrabDevice(ClientPtr client)
                                 &param, XI2, &mask);
             break;
         case XIGrabtypeKeycode:
-            /* XI2 allows 32-bit keycodes but thanks to XKB we can never
-             * implement this. Just return an error for all keycodes that
-             * cannot work anyway */
-            if (stuff->detail > 255)
-                status = XIAlreadyGrabbed;
-            else
-                status = GrabKey(client, dev, mod_dev, stuff->detail,
-                                 &param, XI2, &mask);
+            status = GrabKey(client, dev, mod_dev, stuff->detail,
+                             &param, XI2, &mask);
             break;
         case XIGrabtypeEnter:
         case XIGrabtypeFocusIn:
@@ -334,6 +334,12 @@ ProcXIPassiveUngrabDevice(ClientPtr client)
         return BadValue;
     }
 
+    /* We don't allow passive grabs for details > 255 anyway */
+    if (stuff->detail > 255) {
+        client->errorValue = stuff->detail;
+        return BadValue;
+    }
+
     rc = dixLookupWindow(&win, stuff->grab_window, client, DixSetAttrAccess);
     if (rc != Success)
         return rc;
-- 
2.38.1


++++++ U_0005-Xext-free-the-screen-saver-resource-when-replacing-i.patch ++++++
>From 06eb55528bb62f7418f740152642f2066d593bbf Mon Sep 17 00:00:00 2001
From: Peter Hutterer <peter.hutte...@who-t.net>
Date: Tue, 29 Nov 2022 14:53:07 +1000
Subject: [PATCH xserver 5/6] Xext: free the screen saver resource when
 replacing it

This fixes a use-after-free bug:

When a client first calls ScreenSaverSetAttributes(), a struct
ScreenSaverAttrRec is allocated and added to the client's
resources.

When the same client calls ScreenSaverSetAttributes() again, a new
struct ScreenSaverAttrRec is allocated, replacing the old struct. The
old struct was freed but not removed from the clients resources.

Later, when the client is destroyed the resource system invokes
ScreenSaverFreeAttr and attempts to clean up the already freed struct.

Fix this by letting the resource system free the old attrs instead.

ZDI-CAN 19404

This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative

Signed-off-by: Peter Hutterer <peter.hutte...@who-t.net>
---
 Xext/saver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Xext/saver.c b/Xext/saver.c
index f813ba08d1..fd6153c313 100644
--- a/Xext/saver.c
+++ b/Xext/saver.c
@@ -1051,7 +1051,7 @@ ScreenSaverSetAttributes(ClientPtr client)
         pVlist++;
     }
     if (pPriv->attr)
-        FreeScreenAttr(pPriv->attr);
+        FreeResource(pPriv->attr->resource, AttrType);
     pPriv->attr = pAttr;
     pAttr->resource = FakeClientID(client->index);
     if (!AddResource(pAttr->resource, AttrType, (void *) pAttr))
-- 
2.38.1


++++++ U_0006-Xext-free-the-XvRTVideoNotify-when-turning-off-from-.patch ++++++
>From 4ca304326d3b222a446aca82ec3c28ee8adf8446 Mon Sep 17 00:00:00 2001
From: Peter Hutterer <peter.hutte...@who-t.net>
Date: Wed, 30 Nov 2022 11:20:40 +1000
Subject: [PATCH xserver 6/6] Xext: free the XvRTVideoNotify when turning off
 from the same client

This fixes a use-after-free bug:

When a client first calls XvdiSelectVideoNotify() on a drawable with a
TRUE onoff argument, a struct XvVideoNotifyRec is allocated. This struct
is added twice to the resources:
  - as the drawable's XvRTVideoNotifyList. This happens only once per
    drawable, subsequent calls append to this list.
  - as the client's XvRTVideoNotify. This happens for every client.

The struct keeps the ClientPtr around once it has been added for a
client. The idea, presumably, is that if the client disconnects we can remove
all structs from the drawable's list that match the client (by resetting
the ClientPtr to NULL), but if the drawable is destroyed we can remove
and free the whole list.

However, if the same client then calls XvdiSelectVideoNotify() on the
same drawable with a FALSE onoff argument, only the ClientPtr on the
existing struct was set to NULL. The struct itself remained in the
client's resources.

If the drawable is now destroyed, the resource system invokes
XvdiDestroyVideoNotifyList which frees the whole list for this drawable
- including our struct. This function however does not free the resource
for the client since our ClientPtr is NULL.

Later, when the client is destroyed and the resource system invokes
XvdiDestroyVideoNotify, we unconditionally set the ClientPtr to NULL. On
a struct that has been freed previously. This is generally frowned upon.

Fix this by calling FreeResource() on the second call instead of merely
setting the ClientPtr to NULL. This removes the struct from the client
resources (but not from the list), ensuring that it won't be accessed
again when the client quits.

Note that the assignment tpn->client = NULL; is superfluous since the
XvdiDestroyVideoNotify function will do this anyway. But it's left for
clarity and to match a similar invocation in XvdiSelectPortNotify.

ZDI-CAN 19400

This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative

Signed-off-by: Peter Hutterer <peter.hutte...@who-t.net>
---
 Xext/xvmain.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Xext/xvmain.c b/Xext/xvmain.c
index f627471938..2a08f8744a 100644
--- a/Xext/xvmain.c
+++ b/Xext/xvmain.c
@@ -811,8 +811,10 @@ XvdiSelectVideoNotify(ClientPtr client, DrawablePtr pDraw, 
BOOL onoff)
         tpn = pn;
         while (tpn) {
             if (tpn->client == client) {
-                if (!onoff)
+                if (!onoff) {
                     tpn->client = NULL;
+                    FreeResource(tpn->id, XvRTVideoNotify);
+                }
                 return Success;
             }
             if (!tpn->client)
-- 
2.38.1


++++++ U_0007-xkb-reset-the-radio_groups-pointer-to-NULL-after-fre.patch ++++++
>From 79916ec4eed724b481d24d97686d3ed05a939859 Mon Sep 17 00:00:00 2001
From: Peter Hutterer <peter.hutte...@who-t.net>
Date: Mon, 5 Dec 2022 15:55:54 +1000
Subject: [PATCH xserver] xkb: reset the radio_groups pointer to NULL after
 freeing it

Unlike other elements of the keymap, this pointer was freed but not
reset. On a subsequent XkbGetKbdByName request, the server may access
already freed memory.

ZDI-CAN-19530

This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative

Signed-off-by: Peter Hutterer <peter.hutte...@who-t.net>
---
 xkb/xkbUtils.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/xkb/xkbUtils.c b/xkb/xkbUtils.c
index dd089c2046..3f5791a183 100644
--- a/xkb/xkbUtils.c
+++ b/xkb/xkbUtils.c
@@ -1326,6 +1326,7 @@ _XkbCopyNames(XkbDescPtr src, XkbDescPtr dst)
         }
         else {
             free(dst->names->radio_groups);
+            dst->names->radio_groups = NULL;
         }
         dst->names->num_rg = src->names->num_rg;
 
-- 
2.38.1

Reply via email to