[PATCH 2/4] winex11: Avoid inefficiency and overflow in remove_startup_notification.

Alex Henrie alexhenrie24 at gmail.com
Thu Oct 29 22:30:20 CDT 2015


Cc: Damjan Jovanovic <damjan.jov at gmail.com>
Cc: Vincent Povirk <vincent at codeweavers.com>

Coverity #713245, "Checking pos < 1022U implies that pos is between
1022 and 1023 (inclusive) on the false branch."

The startup notification protocol does not care about anything after the
null terminator, so there is no reason to use memset here. According to
<http://standards.freedesktop.org/startup-notification-spec/startup-notification-latest.txt>:

   The last byte used in the last client message must be nul, and no
   intermediate bytes may be nul. The nul byte identifies
   the end of the message.

Signed-off-by: Alex Henrie <alexhenrie24 at gmail.com>
---
 dlls/winex11.drv/window.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c
index b763677..42aaa17 100644
--- a/dlls/winex11.drv/window.c
+++ b/dlls/winex11.drv/window.c
@@ -126,33 +126,33 @@ static void remove_startup_notification(Display *display, Window window)
 
     if ((src = strstr( id, "_TIME" ))) update_user_time( atol( src + 5 ));
 
-    pos = snprintf(message, sizeof(message), "remove: ID=");
-    message[pos++] = '"';
-    for (i = 0; id[i] && pos < sizeof(message) - 2; i++)
+    pos = sprintf(message, "remove: ID=\"");
+    for (i = 0; id[i]; i++)
     {
         if (id[i] == '"' || id[i] == '\\')
+        {
+            if (pos == sizeof(message) - 3) break;
             message[pos++] = '\\';
+        }
+        if (pos == sizeof(message) - 2) break;
         message[pos++] = id[i];
     }
     message[pos++] = '"';
     message[pos++] = '\0';
 
     xevent.xclient.type = ClientMessage;
     xevent.xclient.message_type = x11drv_atom(_NET_STARTUP_INFO_BEGIN);
     xevent.xclient.display = display;
     xevent.xclient.window = window;
     xevent.xclient.format = 8;
 
     src = message;
-    srclen = strlen(src) + 1;
+    srclen = pos;
 
     while (srclen > 0)
     {
-        int msglen = srclen;
-        if (msglen > 20)
-            msglen = 20;
-        memset(&xevent.xclient.data.b[0], 0, 20);
-        memcpy(&xevent.xclient.data.b[0], src, msglen);
+        int msglen = min(srclen, sizeof(xevent.xclient.data.b));
+        memcpy(xevent.xclient.data.b, src, msglen);
         src += msglen;
         srclen -= msglen;
 
-- 
2.6.2




More information about the wine-patches mailing list