summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAndy Gozas <andy@gozas.me>2022-08-28 22:01:47 +0000
committerAndy Gozas <andy@gozas.me>2022-08-28 22:01:47 +0000
commit513536a47aa495c8f66c0f59b11756a4b72003e1 (patch)
treec3353c8ac789b7dab2eb463253af05d6be3199c8
parentebf6f66fe1b9df9209e1e007622c628520f60f05 (diff)
Fix buffer overflow in input handling
kpress function in x.c previously relied on the wrong understanding of XmbLookupString behavior. When the composed string is longer than the available buffer, the buffer is not initialized and the actual length of the data available to read is returned, not the amount of data written. When that amount is later used to process the contents of this buffer, not only will the random contents of the uninitialized buffer be sent directly to the terminal and whichever application is now running in it, but possibly also whatever is in the memory after that buffer, leading to undefined behavior and possible random command execution.
-rw-r--r--x.c43
1 files changed, 33 insertions, 10 deletions
diff --git a/x.c b/x.c
index 017238f..b5405ed 100644
--- a/x.c
+++ b/x.c
@@ -2018,8 +2018,10 @@ kpress(XEvent *ev)
{
XKeyEvent *e = &ev->xkey;
KeySym ksym;
- char buf[64], *customkey;
- int len;
+ char *buf = NULL, *customkey;
+ int len = 0;
+ int buf_size = 64;
+ int critical = - 1;
Rune c;
Status status;
Shortcut *bp;
@@ -2027,27 +2029,44 @@ kpress(XEvent *ev)
if (IS_SET(MODE_KBDLOCK))
return;
- if (xw.ime.xic)
- len = XmbLookupString(xw.ime.xic, e, buf, sizeof buf, &ksym, &status);
- else
- len = XLookupString(e, buf, sizeof buf, &ksym, NULL);
+reallocbuf:
+ if (critical > 0)
+ goto cleanup;
+ if (buf)
+ free(buf);
+
+ buf = xmalloc((buf_size) * sizeof(char));
+ critical += 1;
+
+ if (xw.ime.xic) {
+ len = XmbLookupString(xw.ime.xic, e, buf, buf_size, &ksym, &status);
+ if (status == XBufferOverflow) {
+ buf_size = len;
+ goto reallocbuf;
+ }
+ } else {
+ // Not sure how to fix this and if it is fixable
+ // but at least it does write something into the buffer
+ // so it is not as critical
+ len = XLookupString(e, buf, buf_size, &ksym, NULL);
+ }
/* 1. shortcuts */
for (bp = shortcuts; bp < shortcuts + LEN(shortcuts); bp++) {
if (ksym == bp->keysym && match(bp->mod, e->state)) {
bp->func(&(bp->arg));
- return;
+ goto cleanup;
}
}
/* 2. custom keys from config.h */
if ((customkey = kmap(ksym, e->state))) {
ttywrite(customkey, strlen(customkey), 1);
- return;
+ goto cleanup;
}
/* 3. composed string from input method */
if (len == 0)
- return;
+ goto cleanup;
if (len == 1 && e->state & Mod1Mask) {
if (IS_SET(MODE_8BIT)) {
if (*buf < 0177) {
@@ -2060,7 +2079,11 @@ kpress(XEvent *ev)
len = 2;
}
}
- ttywrite(buf, len, 1);
+ if (len <= buf_size)
+ ttywrite(buf, len, 1);
+cleanup:
+ if (buf)
+ free(buf);
}
void