From c33f2ad355a291cb1a919074ceaa25f52bc85b76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bert=20M=C3=BCnnich?= Date: Wed, 1 Oct 2014 22:35:22 +0200 Subject: [PATCH] Corrected & refactored handling of window bar content... Old snprintf calls could have overflowed the buffers. --- main.c | 66 +++++++++++++++++++++++++++++++++----------------------- window.c | 32 ++++++++++++++++----------- window.h | 10 +++++++-- 3 files changed, 67 insertions(+), 41 deletions(-) diff --git a/main.c b/main.c index 31effbd..46d80aa 100644 --- a/main.c +++ b/main.c @@ -21,6 +21,7 @@ #include #include +#include #include #include #include @@ -256,7 +257,7 @@ void open_info(void) kill(pid, SIGTERM); info.fd = -1; } - win.bar.l[0] = '\0'; + win.bar.l.buf[0] = '\0'; if (pipe(pfd) < 0) return; @@ -290,20 +291,20 @@ void read_info(void) for (i = 0; i < n; i++) { if (buf[i] == '\n') { if (info.lastsep == 0) { - win.bar.l[info.i++] = ' '; + win.bar.l.buf[info.i++] = ' '; info.lastsep = 1; } } else { - win.bar.l[info.i++] = buf[i]; + win.bar.l.buf[info.i++] = buf[i]; info.lastsep = 0; } - if (info.i + 1 == sizeof(win.bar.l)) + if (info.i + 1 == win.bar.l.size) goto end; } } end: info.i -= info.lastsep; - win.bar.l[info.i] = '\0'; + win.bar.l.buf[info.i] = '\0'; win_draw(&win); close(info.fd); info.fd = -1; @@ -343,15 +344,24 @@ void load_image(int new) reset_timeout(animate); } +void bar_put(win_bar_t *bar, const char *fmt, ...) +{ + size_t len = bar->size - (bar->p - bar->buf), n; + va_list ap; + + va_start(ap, fmt); + n = vsnprintf(bar->p, len, fmt, ap); + bar->p += MIN(len, n); + va_end(ap); +} + void update_info(void) { - unsigned int i, fn, fw, n; - unsigned int llen = sizeof(win.bar.l), rlen = sizeof(win.bar.r); - char *lt = win.bar.l, *rt = win.bar.r, title[TITLE_LEN]; + unsigned int i, fn, fw; + char title[TITLE_LEN]; const char * mark; bool ow_info; - - for (fw = 0, i = filecnt; i > 0; fw++, i /= 10); + win_bar_t *l = &win.bar.l, *r = &win.bar.r; /* update window title */ if (mode == MODE_THUMB) { @@ -364,39 +374,41 @@ void update_info(void) /* update bar contents */ if (win.bar.h == 0) return; + for (fw = 0, i = filecnt; i > 0; fw++, i /= 10); mark = files[fileidx].marked ? "* " : ""; + l->p = l->buf; + r->p = r->buf; if (mode == MODE_THUMB) { if (tns.loadnext < tns.end) { - snprintf(lt, llen, "Loading... %0*d", fw, tns.loadnext); + bar_put(l, "Loading... %0*d", fw, MAX(tns.loadnext, 1)); ow_info = false; } else { ow_info = true; } - n = snprintf(rt, rlen, "%s%0*d/%d", mark, fw, fileidx + 1, filecnt); + bar_put(r, "%s%0*d/%d", mark, fw, fileidx + 1, filecnt); } else { - n = snprintf(rt, rlen, "%s", mark); + bar_put(r, "%s", mark); if (img.ss.on) - n += snprintf(rt + n, rlen - n, "%ds | ", img.ss.delay); + bar_put(r, "%ds | ", img.ss.delay); if (img.gamma != 0) - n += snprintf(rt + n, rlen - n, "G%+d | ", img.gamma); - n += snprintf(rt + n, rlen - n, "%3d%% | ", (int) (img.zoom * 100.0)); + bar_put(r, "G%+d | ", img.gamma); + bar_put(r, "%3d%% | ", (int) (img.zoom * 100.0)); if (img.multi.cnt > 0) { for (fn = 0, i = img.multi.cnt; i > 0; fn++, i /= 10); - n += snprintf(rt + n, rlen - n, "%0*d/%d | ", - fn, img.multi.sel + 1, img.multi.cnt); + bar_put(r, "%0*d/%d | ", fn, img.multi.sel + 1, img.multi.cnt); } - n += snprintf(rt + n, rlen - n, "%0*d/%d", fw, fileidx + 1, filecnt); + bar_put(r, "%0*d/%d", fw, fileidx + 1, filecnt); ow_info = info.cmd == NULL; } if (ow_info) { fn = strlen(files[fileidx].name); - if (fn < llen && + if (fn < l->size && win_textwidth(files[fileidx].name, fn, true) + - win_textwidth(rt, n, true) < win.w) + win_textwidth(r->buf, r->p - r->buf, true) < win.w) { - strncpy(lt, files[fileidx].name, llen); + strncpy(l->buf, files[fileidx].name, l->size); } else { - strncpy(lt, files[fileidx].base, llen); + strncpy(l->buf, files[fileidx].base, l->size); } } } @@ -469,7 +481,7 @@ void run_key_handler(const char *key, unsigned int mask) int i, j, retval, status; int fcnt = mode == MODE_THUMB && markcnt > 0 ? markcnt : 1; bool changed = false; - char **args, kstr[32], oldbar[sizeof(win.bar.l)]; + char **args, kstr[32], oldbar[BAR_L_LEN]; struct stat *oldst, newst; struct { int fn; struct stat st; } *finfo; @@ -504,8 +516,8 @@ void run_key_handler(const char *key, unsigned int mask) mask & Mod1Mask ? "M-" : "", mask & ShiftMask ? "S-" : "", key); - memcpy(oldbar, win.bar.l, sizeof(win.bar.l)); - strncpy(win.bar.l, "Running key handler...", sizeof(win.bar.l)); + memcpy(oldbar, win.bar.l.buf, sizeof(oldbar)); + strncpy(win.bar.l.buf, "Running key handler...", win.bar.l.size); win_draw(&win); win_set_cursor(&win, CURSOR_WATCH); @@ -540,7 +552,7 @@ end: img_close(&img, true); load_image(fileidx); } else if (info.cmd != NULL) { - memcpy(win.bar.l, oldbar, sizeof(win.bar.l)); + memcpy(win.bar.l.buf, oldbar, win.bar.l.size); } } reset_cursor(); diff --git a/window.c b/window.c index 581ca76..ff7f116 100644 --- a/window.c +++ b/window.c @@ -168,7 +168,12 @@ void win_init(win_t *win) win->selcol = win_alloc_color(win, SEL_COLOR); win->bar.bgcol = win_alloc_color(win, BAR_BG_COLOR); win->bar.fgcol = win_alloc_color(win, BAR_FG_COLOR); - win->bar.h = options->hide_bar ? 0 : barheight; + + win->bar.l.size = BAR_L_LEN; + win->bar.r.size = BAR_R_LEN; + win->bar.l.buf = s_malloc(win->bar.l.size); + win->bar.r.buf = s_malloc(win->bar.r.size); + win->bar.h = options->hide_bar ? 0 : barheight; INIT_ATOM_(WM_DELETE_WINDOW); INIT_ATOM_(_NET_WM_NAME); @@ -416,9 +421,12 @@ void win_draw_bar(win_t *win) char rest[3]; const char *dots = "..."; win_env_t *e; + win_bar_t *l, *r; if (win == NULL || win->xwin == None) return; + if ((l = &win->bar.l)->buf == NULL || (r = &win->bar.r)->buf == NULL) + return; e = &win->env; y = win->h + font.ascent + V_TEXT_PAD; @@ -430,35 +438,35 @@ void win_draw_bar(win_t *win) XSetForeground(e->dpy, gc, win->bar.fgcol); XSetBackground(e->dpy, gc, win->bar.bgcol); - if ((len = strlen(win->bar.r)) > 0) { - if ((tw = win_textwidth(win->bar.r, len, true)) > w) + if ((len = strlen(r->buf)) > 0) { + if ((tw = win_textwidth(r->buf, len, true)) > w) return; x = win->w - tw + H_TEXT_PAD; w -= tw; if (font.set) - XmbDrawString(e->dpy, win->buf.pm, font.set, gc, x, y, win->bar.r, len); + XmbDrawString(e->dpy, win->buf.pm, font.set, gc, x, y, r->buf, len); else - XDrawString(e->dpy, win->buf.pm, gc, x, y, win->bar.r, len); + XDrawString(e->dpy, win->buf.pm, gc, x, y, r->buf, len); } - if ((len = strlen(win->bar.l)) > 0) { + if ((len = strlen(l->buf)) > 0) { olen = len; - while (len > 0 && (tw = win_textwidth(win->bar.l, len, true)) > w) + while (len > 0 && (tw = win_textwidth(l->buf, len, true)) > w) len--; if (len > 0) { if (len != olen) { w = strlen(dots); if (len <= w) return; - memcpy(rest, win->bar.l + len - w, w); - memcpy(win->bar.l + len - w, dots, w); + memcpy(rest, l->buf + len - w, w); + memcpy(l->buf + len - w, dots, w); } x = H_TEXT_PAD; if (font.set) - XmbDrawString(e->dpy, win->buf.pm, font.set, gc, x, y, win->bar.l, len); + XmbDrawString(e->dpy, win->buf.pm, font.set, gc, x, y, l->buf, len); else - XDrawString(e->dpy, win->buf.pm, gc, x, y, win->bar.l, len); + XDrawString(e->dpy, win->buf.pm, gc, x, y, l->buf, len); if (len != olen) - memcpy(win->bar.l + len - w, rest, w); + memcpy(l->buf + len - w, rest, w); } } } diff --git a/window.h b/window.h index 1178007..8761120 100644 --- a/window.h +++ b/window.h @@ -49,6 +49,12 @@ typedef struct { int depth; } win_env_t; +typedef struct { + size_t size; + char *p; + char *buf; +} win_bar_t; + typedef struct { Window xwin; win_env_t env; @@ -73,8 +79,8 @@ typedef struct { struct { unsigned int h; - char l[BAR_L_LEN]; - char r[BAR_R_LEN]; + win_bar_t l; + win_bar_t r; unsigned long bgcol; unsigned long fgcol; } bar;