Tk Source Code

When Tk is compiled with XCode 8.3.1, a buffer overflow check mechanism inside the compiler crashes Tk at "wm protocol". 

Script to reproduce:

package require Tk
wm protocol . WM_DELETE_WINDOW justsomething


Cause: 
tkMacOSXWM.h defines a variable length structure with a fixed field, which trips up the overflow detector. 

Fix: 

diff -ru tk8.6/macosx/tkMacOSXWm.h tk8.6-patched/macosx/tkMacOSXWm.h
--- tk8.6/macosx/tkMacOSXWm.h	2017-04-06 18:11:56.000000000 +0200
+++ tk8.6-patched/macosx/tkMacOSXWm.h	2017-04-08 15:03:44.000000000 +0200
@@ -29,7 +29,7 @@
 				 * same top-level window, or NULL for end of
 				 * list. */
     Tcl_Interp *interp;		/* Interpreter in which to invoke command. */
-    char command[4];		/* Tcl command to invoke when a client message
+    char command[];		/* Tcl command to invoke when a client message
 				 * for this protocol arrives. The actual size
 				 * of the structure varies to accommodate the
 				 * needs of the actual command. THIS MUST BE
@@ -37,7 +37,7 @@
 } ProtocolHandler;
 
 #define HANDLER_SIZE(cmdLength) \
-((unsigned) (sizeof(ProtocolHandler) - 3 + cmdLength))
+((unsigned) (sizeof(ProtocolHandler) + cmdLength + 1))
 
 /*
  * A data structure of the following type holds window-manager-related

Merged to core-8-6-branch and trunk.

1. OK, let Linux and Win be unchanged then since the code in itself is not buggy and since the existing code is C89-compatible code in case anyone needs this (just wondering who...)

2. I had a bug crashing the test suite on OS X but never had time to dig into it. Now I have checked that before your fix it crashes in wm-protocol-2.1, and it crashes with the call stack you showed me in the chat. And with your fix this does not happen anymore.

I think this can now be merged. I won't wait for the usual week because I need this in other branches. Well done, many thanks again!

Thanks for taking a look. 

1. The original code is not in itself buggy; it is just the overzealous buffer overflow detector which triggers (a false alarm). The corrected code uses a C99 feature (char[]). I don't know whether C99 is a problem on some Unix platforms - certainly on OSX it is impossible to use a compiler without C99 support. It's therefore left to the maintainers to decide for Unix/Windows.

2. To trigger the bug, the script must be longer than 4 chars, which is tested in the wm tests. I haven't checked that the test indeed triggers the bug, though.

Forget to state it: the patch is in branch bug-3f323bf2b4.

Thanks for the report and patch (which indeed fixes this issue).

Two questions:

1. Don't we have similar issue, on Linux and on Win?

2. Should we add a testcase or do you think the existing test suite for wm is enough?