[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#990647: nxagent: in Compext.c: fix comparisons of 16bit sequence numbers



Package: nxagent
Severity: important
Version: 3.5.99.26-1
Tags: patch

In Compext.c a flawed comparison has been discovered lately by Ulrich Sibiller and Norm Green.

From the commit message:

```
rep->generic.sequenceNumber is of type CARD16
state->sequence is of type unsigned long

Converting state->sequence to an int as it has been done since the
first version of nxcomp I know of (1.3.0-18 from 2003) is wrong here
because for numbers > INT_MAX this will result in a negative number,
which, after applying the 16bit modulo, will not match
rep->generic.sequenceNumber.

Example with numbers:

CARD16 c = 24565
unsigned long u = 3179110389

c % 65536 = 24565
u % 65536 = 24565

(int)(u) = -1115856907
(int)(u) % 65536 = -40971

-40971 will not match 24565

To fix this we need to ensure the number stays positive. We use CARD16
for this to match the type in the request which is a 16bit number. On
my system CARD16 is unsigned short which is guaranteed to contain _at
least_ the 0-65,535 range. As there is no upper limit of the range we
cannot drop the modulo because we need this value to be 16bit and not
more.

Thanks to Norm Green for providing log after log until we could
finally identify the reason for him seeing "Xlib: unexpected async
reply (sequence 0x94b01439)!" when pasting stopped working.
```

Patch for this has been attached.

Mike
--

DAS-NETZWERKTEAM
c\o Technik- und Ökologiezentrum Eckernförde
Mike Gabriel, Marienthaler Str. 17, 24340 Eckernförde
mobile: +49 (1520) 1976 148
landline: +49 (4351) 850 8940

GnuPG Fingerprint: 9BFB AEE8 6C0A A5FF BF22  0782 9AF4 6B30 2577 1B31
mail: mike.gabriel@das-netzwerkteam.de, http://das-netzwerkteam.de

>From 1b4ebce2ce8ef29c01bd124ed56c9d6a14c9a82d Mon Sep 17 00:00:00 2001
From: Ulrich Sibiller <uli42@gmx.de>
Date: Wed, 17 Mar 2021 22:17:55 +0100
Subject: [PATCH] Compext.c: fix comparisons of 16bit sequence numbers

rep->generic.sequenceNumber is of type CARD16
state->sequence is of type unsigned long

Converting state->sequence to an int as it has been done since the
first version of nxcomp I know of (1.3.0-18 from 2003) is wrong here
because for numbers > INT_MAX this will result in a negative number,
which, after applying the 16bit modulo, will not match
rep->generic.sequenceNumber.

Example with numbers:

CARD16 c = 24565
unsigned long u = 3179110389

c % 65536 = 24565
u % 65536 = 24565

(int)(u) = -1115856907
(int)(u) % 65536 = -40971

-40971 will not match 24565

To fix this we need to ensure the number stays positive. We use CARD16
for this to match the type in the request which is a 16bit number. On
my system CARD16 is unsigned short which is guaranteed to contain _at
least_ the 0-65,535 range. As there is no upper limit of the range we
cannot drop the modulo because we need this value to be 16bit and not
more.

Thanks to Norm Green for providing log after log until we could
finally identify the reason for him seeing "Xlib: unexpected async
reply (sequence 0x94b01439)!" when pasting stopped working.

Signed-off-by: Mike Gabriel <mike.gabriel@das-netzwerkteam.de>
---
 nx-X11/programs/Xserver/hw/nxagent/compext/Compext.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/nx-X11/programs/Xserver/hw/nxagent/compext/Compext.c b/nx-X11/programs/Xserver/hw/nxagent/compext/Compext.c
index 4a8dacaf4..7a6cb9e30 100644
--- a/nx-X11/programs/Xserver/hw/nxagent/compext/Compext.c
+++ b/nx-X11/programs/Xserver/hw/nxagent/compext/Compext.c
@@ -3435,7 +3435,7 @@ static Bool _NXCollectImageHandler(Display *dpy, xReply *rep, char *buf,
   state = (_NXCollectImageState *) data;
 
   if ((rep -> generic.sequenceNumber % 65536) !=
-          ((int)(state -> sequence) % 65536))
+          ((CARD16)(state -> sequence) % 65536))
   {
     #ifdef TEST
     fprintf(stderr, "******_NXCollectImageHandler: Unmatched sequence [%d] for opcode [%d] "
@@ -3819,7 +3819,7 @@ static Bool _NXCollectPropertyHandler(Display *dpy, xReply *rep, char *buf,
   state = (_NXCollectPropertyState *) data;
 
   if ((rep -> generic.sequenceNumber % 65536) !=
-          ((int)(state -> sequence) % 65536))
+          ((CARD16)(state -> sequence) % 65536))
   {
     #ifdef TEST
     fprintf(stderr, "******_NXCollectPropertyHandler: Unmatched sequence [%d] for opcode [%d] "
@@ -4173,7 +4173,7 @@ static Bool _NXCollectGrabPointerHandler(Display *dpy, xReply *rep, char *buf,
   state = (_NXCollectGrabPointerState *) data;
 
   if ((rep -> generic.sequenceNumber % 65536) !=
-          ((int)(state -> sequence) % 65536))
+          ((CARD16)(state -> sequence) % 65536))
   {
     #ifdef TEST
     fprintf(stderr, "******_NXCollectGrabPointerHandler: Unmatched sequence [%d] for opcode [%d] "
@@ -4447,7 +4447,7 @@ static Bool _NXCollectInputFocusHandler(Display *dpy, xReply *rep, char *buf,
   state = (_NXCollectInputFocusState *) data;
 
   if ((rep -> generic.sequenceNumber % 65536) !=
-          ((int)(state -> sequence) % 65536))
+          ((CARD16)(state -> sequence) % 65536))
   {
     #ifdef TEST
     fprintf(stderr, "******_NXCollectInputFocusHandler: Unmatched sequence [%d] for opcode [%d] "
-- 
2.30.2

Attachment: pgpmJ2ldyL6is.pgp
Description: Digitale PGP-Signatur


Reply to: