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

[PATCH] Fix buffer overflows in shortcut and workspace name handling



Fix buffer overflows in shortcut and workspace name handling

The handling of user defined shortcuts was not checking the length
of the shortcut before copying it to a fixed-length temporary buffer,

char buf[128];

     strcpy(buf, shortcutDefinition);

and strcpy() is well known for not checking if overflows will occur.

In particular, wmaker was crashing here if a big 'shortcut' was defined
either through WPrefs or by directly editing the configuration files.

This is now avoided by using strncpy() instead, and wmaker does not
crash anymore.

And this patch also fixes a similar buffer overflow for big workspace
names too.

Furthermore, use MAX_SHORTCUT_LENGTH instead of raw number and define
it to be 32 instead of 128.
---

John: I screwed up my Mercurial repo beyond repair.
So I am sending the patch ('git format-patch HEAD~1') from my git repo 
instead and I hope it will apply just fine.

I noticed that src/wconfig.h.in is in .hgignore (or .cvsignore) and
therefore I did not find a better place to put the #define than
writing them on each file, which is ugly.

BTW, I think having src/wconfig.h.in in .hgignore is a mistake.

 src/defaults.c  |    6 +++---
 src/rootmenu.c  |    6 +++---
 src/usermenu.c  |    8 +++++---
 src/workspace.c |    3 ++-
 4 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/src/defaults.c b/src/defaults.c
index 7bc18e0..d7fbb60 100644
--- a/src/defaults.c
+++ b/src/defaults.c
@@ -69,7 +69,7 @@
 #include "workspace.h"
 #include "properties.h"
 
-
+#define MAX_SHORTCUT_LENGTH 32
 
 /***** Global *****/
 
@@ -2515,7 +2515,7 @@ getKeybind(WScreen *scr, WDefaultEntry *entry, WMPropList *value, void *addr,
     KeySym ksym;
     char *val;
     char *k;
-    char buf[128], *b;
+    char buf[MAX_SHORTCUT_LENGTH], *b;
 
 
     GET_STRING_OR_DEFAULT("Key spec", val);
@@ -2528,7 +2528,7 @@ getKeybind(WScreen *scr, WDefaultEntry *entry, WMPropList *value, void *addr,
         return True;
     }
 
-    strcpy(buf, val);
+    strncpy(buf, val, MAX_SHORTCUT_LENGTH);
 
     b = (char*)buf;
 
diff --git a/src/rootmenu.c b/src/rootmenu.c
index 58eedb2..be1bbed 100644
--- a/src/rootmenu.c
+++ b/src/rootmenu.c
@@ -55,7 +55,7 @@
 
 #include <WINGs/WUtil.h>
 
-
+#define MAX_SHORTCUT_LENGTH 32
 
 extern char *Locale;
 
@@ -513,11 +513,11 @@ addShortcut(char *file, char *shortcutDefinition, WMenu *menu,
     Shortcut *ptr;
     KeySym ksym;
     char *k;
-    char buf[128], *b;
+    char buf[MAX_SHORTCUT_LENGTH], *b;
 
     ptr = wmalloc(sizeof(Shortcut));
 
-    strcpy(buf, shortcutDefinition);
+    strncpy(buf, shortcutDefinition, MAX_SHORTCUT_LENGTH);
     b = (char*)buf;
 
     /* get modifiers */
diff --git a/src/usermenu.c b/src/usermenu.c
index d1a7765..55ee51b 100644
--- a/src/usermenu.c
+++ b/src/usermenu.c
@@ -79,6 +79,7 @@
 
 #include "framewin.h"
 
+#define MAX_SHORTCUT_LENGTH 32
 
 /*** var ***/
 extern WPreferences wPreferences;
@@ -139,7 +140,7 @@ convertShortcuts(WScreen *scr, WMPropList *shortcut)
     KeySym ksym;
     char *k;
     char *buffer;
-    char buf[128], *b;
+    char buf[MAX_SHORTCUT_LENGTH], *b;
     int keycount,i,j,mod;
 
     if (WMIsPLString(shortcut)) {
@@ -163,9 +164,10 @@ convertShortcuts(WScreen *scr, WMPropList *shortcut)
     for (i=0,j=0;i<keycount;i++) {
         data->key[j].modifier = 0;
         if (WMIsPLArray(shortcut)) {
-            strcpy(buf, WMGetFromPLString(WMGetFromPLArray(shortcut, i)));
+            strncpy(buf, WMGetFromPLString(WMGetFromPLArray(shortcut, i)),
+		    MAX_SHORTCUT_LENGTH);
         } else {
-            strcpy(buf, WMGetFromPLString(shortcut));
+            strncpy(buf, WMGetFromPLString(shortcut), MAX_SHORTCUT_LENGTH);
         }
         b = (char*)buf;
 
diff --git a/src/workspace.c b/src/workspace.c
index 3c72710..4649bbe 100644
--- a/src/workspace.c
+++ b/src/workspace.c
@@ -54,6 +54,7 @@
 
 #include "xinerama.h"
 
+#define MAX_SHORTCUT_LENGTH 32
 
 extern WPreferences wPreferences;
 extern XContext wWinContext;
@@ -1384,7 +1385,7 @@ wWorkspaceMenuUpdate(WScreen *scr, WMenu *menu)
         i = scr->workspace_count-(menu->entry_no-2);
         ws = menu->entry_no - 2;
         while (i>0) {
-            strcpy(title, scr->workspaces[ws]->name);
+            strncpy(title, scr->workspaces[ws]->name, MAX_WORKSPACENAME_WIDTH);
 
             entry = wMenuAddCallback(menu, title, switchWSCommand, (void*)ws);
             entry->flags.indicator = 1;
-- 
1.6.0.3.618.g55080


-- 
To unsubscribe, send mail to wmaker-dev-unsubscribe@lists.windowmaker.info.