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.
5 files changed, 10 insertions(+), 8 deletions(-)
src/defaults.c | 4 ++--
src/rootmenu.c | 4 ++--
src/usermenu.c | 7 ++++---
src/wconfig.h.in | 1 +
src/workspace.c | 2 +-
# HG changeset patch
# User Carlos R. Mafra <[email protected]>
# Date 1229313545 28800
# Node ID b4aba6af63693169dceb439cc144e498bfac7a9c
# Parent 9e5a9405eb7d2123eb5682a9bd9147d22421bf10
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.
diff --git a/src/defaults.c b/src/defaults.c
--- a/src/defaults.c
+++ b/src/defaults.c
@@ -2517,7 +2517,7 @@
KeySym ksym;
char *val;
char *k;
- char buf[128], *b;
+ char buf[MAX_SHORTCUT_LENGTH], *b;
GET_STRING_OR_DEFAULT("Key spec", val);
@@ -2530,7 +2530,7 @@
return True;
}
- strcpy(buf, val);
+ strncpy(buf, val, MAX_SHORTCUT_LENGTH);
b = (char*)buf;
diff --git a/src/rootmenu.c b/src/rootmenu.c
--- a/src/rootmenu.c
+++ b/src/rootmenu.c
@@ -513,11 +513,11 @@
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
--- a/src/usermenu.c
+++ b/src/usermenu.c
@@ -139,7 +139,7 @@
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 +163,10 @@
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/wconfig.h.in b/src/wconfig.h.in
--- a/src/wconfig.h.in
+++ b/src/wconfig.h.in
@@ -489,6 +489,7 @@
#define WM_PI 3.14159265358979323846
+#define MAX_SHORTCUT_LENGTH 32
#define FRAME_BORDER_WIDTH 1 /* width of window border for frames */
#define RESIZEBAR_HEIGHT 8 /* height of the resizebar */
diff --git a/src/workspace.c b/src/workspace.c
--- a/src/workspace.c
+++ b/src/workspace.c
@@ -1384,7 +1384,7 @@
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;
