Krolyxon
Commit c33f2ad

Branches

Refs →

nsxivc33f2ad

Corrected & refactored handling of window bar content...

Old snprintf calls could have overflowed the buffers.
Author
Bert Münnich
Date
2014-10-01 22:35 +0200
Parent
8db3191
Commit
c33f2ad355a291cb1a919074ceaa25f52bc85b76

+67 −41

3 files changed

Browse files Patch
−Context 3+
  1. M main.c +39 −27
  2. M window.c +20 −12
  3. M window.h +8 −2
M main.c View
@@ -21,6 +21,7 @@
2121
2222 #include <stdlib.h>
2323 #include <stdio.h>
24+#include <stdarg.h>
2425 #include <string.h>
2526 #include <fcntl.h>
2627 #include <unistd.h>
@@ -256,7 +257,7 @@ void open_info(void)
256257 kill(pid, SIGTERM);
257258 info.fd = -1;
258259 }
259− win.bar.l[0] = '\0';
260+ win.bar.l.buf[0] = '\0';
260261
261262 if (pipe(pfd) < 0)
262263 return;
@@ -290,20 +291,20 @@ void read_info(void)
290291 for (i = 0; i < n; i++) {
291292 if (buf[i] == '\n') {
292293 if (info.lastsep == 0) {
293− win.bar.l[info.i++] = ' ';
294+ win.bar.l.buf[info.i++] = ' ';
294295 info.lastsep = 1;
295296 }
296297 } else {
297− win.bar.l[info.i++] = buf[i];
298+ win.bar.l.buf[info.i++] = buf[i];
298299 info.lastsep = 0;
299300 }
300− if (info.i + 1 == sizeof(win.bar.l))
301+ if (info.i + 1 == win.bar.l.size)
301302 goto end;
302303 }
303304 }
304305 end:
305306 info.i -= info.lastsep;
306− win.bar.l[info.i] = '\0';
307+ win.bar.l.buf[info.i] = '\0';
307308 win_draw(&win);
308309 close(info.fd);
309310 info.fd = -1;
@@ -343,15 +344,24 @@ void load_image(int new)
343344 reset_timeout(animate);
344345 }
345346
347+void bar_put(win_bar_t *bar, const char *fmt, ...)
348+{
349+ size_t len = bar->size - (bar->p - bar->buf), n;
350+ va_list ap;
351+
352+ va_start(ap, fmt);
353+ n = vsnprintf(bar->p, len, fmt, ap);
354+ bar->p += MIN(len, n);
355+ va_end(ap);
356+}
357+
346358 void update_info(void)
347359 {
348− unsigned int i, fn, fw, n;
349− unsigned int llen = sizeof(win.bar.l), rlen = sizeof(win.bar.r);
350− char *lt = win.bar.l, *rt = win.bar.r, title[TITLE_LEN];
360+ unsigned int i, fn, fw;
361+ char title[TITLE_LEN];
351362 const char * mark;
352363 bool ow_info;
353−
354− for (fw = 0, i = filecnt; i > 0; fw++, i /= 10);
364+ win_bar_t *l = &win.bar.l, *r = &win.bar.r;
355365
356366 /* update window title */
357367 if (mode == MODE_THUMB) {
@@ -364,39 +374,41 @@ void update_info(void)
364374 /* update bar contents */
365375 if (win.bar.h == 0)
366376 return;
377+ for (fw = 0, i = filecnt; i > 0; fw++, i /= 10);
367378 mark = files[fileidx].marked ? "* " : "";
379+ l->p = l->buf;
380+ r->p = r->buf;
368381 if (mode == MODE_THUMB) {
369382 if (tns.loadnext < tns.end) {
370− snprintf(lt, llen, "Loading... %0*d", fw, tns.loadnext);
383+ bar_put(l, "Loading... %0*d", fw, MAX(tns.loadnext, 1));
371384 ow_info = false;
372385 } else {
373386 ow_info = true;
374387 }
375− n = snprintf(rt, rlen, "%s%0*d/%d", mark, fw, fileidx + 1, filecnt);
388+ bar_put(r, "%s%0*d/%d", mark, fw, fileidx + 1, filecnt);
376389 } else {
377− n = snprintf(rt, rlen, "%s", mark);
390+ bar_put(r, "%s", mark);
378391 if (img.ss.on)
379− n += snprintf(rt + n, rlen - n, "%ds | ", img.ss.delay);
392+ bar_put(r, "%ds | ", img.ss.delay);
380393 if (img.gamma != 0)
381− n += snprintf(rt + n, rlen - n, "G%+d | ", img.gamma);
382− n += snprintf(rt + n, rlen - n, "%3d%% | ", (int) (img.zoom * 100.0));
394+ bar_put(r, "G%+d | ", img.gamma);
395+ bar_put(r, "%3d%% | ", (int) (img.zoom * 100.0));
383396 if (img.multi.cnt > 0) {
384397 for (fn = 0, i = img.multi.cnt; i > 0; fn++, i /= 10);
385− n += snprintf(rt + n, rlen - n, "%0*d/%d | ",
386− fn, img.multi.sel + 1, img.multi.cnt);
398+ bar_put(r, "%0*d/%d | ", fn, img.multi.sel + 1, img.multi.cnt);
387399 }
388− n += snprintf(rt + n, rlen - n, "%0*d/%d", fw, fileidx + 1, filecnt);
400+ bar_put(r, "%0*d/%d", fw, fileidx + 1, filecnt);
389401 ow_info = info.cmd == NULL;
390402 }
391403 if (ow_info) {
392404 fn = strlen(files[fileidx].name);
393− if (fn < llen &&
405+ if (fn < l->size &&
394406 win_textwidth(files[fileidx].name, fn, true) +
395− win_textwidth(rt, n, true) < win.w)
407+ win_textwidth(r->buf, r->p - r->buf, true) < win.w)
396408 {
397− strncpy(lt, files[fileidx].name, llen);
409+ strncpy(l->buf, files[fileidx].name, l->size);
398410 } else {
399− strncpy(lt, files[fileidx].base, llen);
411+ strncpy(l->buf, files[fileidx].base, l->size);
400412 }
401413 }
402414 }
@@ -469,7 +481,7 @@ void run_key_handler(const char *key, unsigned int mask)
469481 int i, j, retval, status;
470482 int fcnt = mode == MODE_THUMB && markcnt > 0 ? markcnt : 1;
471483 bool changed = false;
472− char **args, kstr[32], oldbar[sizeof(win.bar.l)];
484+ char **args, kstr[32], oldbar[BAR_L_LEN];
473485 struct stat *oldst, newst;
474486 struct { int fn; struct stat st; } *finfo;
475487
@@ -504,8 +516,8 @@ void run_key_handler(const char *key, unsigned int mask)
504516 mask & Mod1Mask ? "M-" : "",
505517 mask & ShiftMask ? "S-" : "", key);
506518
507− memcpy(oldbar, win.bar.l, sizeof(win.bar.l));
508− strncpy(win.bar.l, "Running key handler...", sizeof(win.bar.l));
519+ memcpy(oldbar, win.bar.l.buf, sizeof(oldbar));
520+ strncpy(win.bar.l.buf, "Running key handler...", win.bar.l.size);
509521 win_draw(&win);
510522 win_set_cursor(&win, CURSOR_WATCH);
511523
@@ -540,7 +552,7 @@ end:
540552 img_close(&img, true);
541553 load_image(fileidx);
542554 } else if (info.cmd != NULL) {
543− memcpy(win.bar.l, oldbar, sizeof(win.bar.l));
555+ memcpy(win.bar.l.buf, oldbar, win.bar.l.size);
544556 }
545557 }
546558 reset_cursor();
M window.c View
@@ -168,7 +168,12 @@ void win_init(win_t *win)
168168 win->selcol = win_alloc_color(win, SEL_COLOR);
169169 win->bar.bgcol = win_alloc_color(win, BAR_BG_COLOR);
170170 win->bar.fgcol = win_alloc_color(win, BAR_FG_COLOR);
171− win->bar.h = options->hide_bar ? 0 : barheight;
171+
172+ win->bar.l.size = BAR_L_LEN;
173+ win->bar.r.size = BAR_R_LEN;
174+ win->bar.l.buf = s_malloc(win->bar.l.size);
175+ win->bar.r.buf = s_malloc(win->bar.r.size);
176+ win->bar.h = options->hide_bar ? 0 : barheight;
172177
173178 INIT_ATOM_(WM_DELETE_WINDOW);
174179 INIT_ATOM_(_NET_WM_NAME);
@@ -416,9 +421,12 @@ void win_draw_bar(win_t *win)
416421 char rest[3];
417422 const char *dots = "...";
418423 win_env_t *e;
424+ win_bar_t *l, *r;
419425
420426 if (win == NULL || win->xwin == None)
421427 return;
428+ if ((l = &win->bar.l)->buf == NULL || (r = &win->bar.r)->buf == NULL)
429+ return;
422430
423431 e = &win->env;
424432 y = win->h + font.ascent + V_TEXT_PAD;
@@ -430,35 +438,35 @@ void win_draw_bar(win_t *win)
430438 XSetForeground(e->dpy, gc, win->bar.fgcol);
431439 XSetBackground(e->dpy, gc, win->bar.bgcol);
432440
433− if ((len = strlen(win->bar.r)) > 0) {
434− if ((tw = win_textwidth(win->bar.r, len, true)) > w)
441+ if ((len = strlen(r->buf)) > 0) {
442+ if ((tw = win_textwidth(r->buf, len, true)) > w)
435443 return;
436444 x = win->w - tw + H_TEXT_PAD;
437445 w -= tw;
438446 if (font.set)
439− XmbDrawString(e->dpy, win->buf.pm, font.set, gc, x, y, win->bar.r, len);
447+ XmbDrawString(e->dpy, win->buf.pm, font.set, gc, x, y, r->buf, len);
440448 else
441− XDrawString(e->dpy, win->buf.pm, gc, x, y, win->bar.r, len);
449+ XDrawString(e->dpy, win->buf.pm, gc, x, y, r->buf, len);
442450 }
443− if ((len = strlen(win->bar.l)) > 0) {
451+ if ((len = strlen(l->buf)) > 0) {
444452 olen = len;
445− while (len > 0 && (tw = win_textwidth(win->bar.l, len, true)) > w)
453+ while (len > 0 && (tw = win_textwidth(l->buf, len, true)) > w)
446454 len--;
447455 if (len > 0) {
448456 if (len != olen) {
449457 w = strlen(dots);
450458 if (len <= w)
451459 return;
452− memcpy(rest, win->bar.l + len - w, w);
453− memcpy(win->bar.l + len - w, dots, w);
460+ memcpy(rest, l->buf + len - w, w);
461+ memcpy(l->buf + len - w, dots, w);
454462 }
455463 x = H_TEXT_PAD;
456464 if (font.set)
457− XmbDrawString(e->dpy, win->buf.pm, font.set, gc, x, y, win->bar.l, len);
465+ XmbDrawString(e->dpy, win->buf.pm, font.set, gc, x, y, l->buf, len);
458466 else
459− XDrawString(e->dpy, win->buf.pm, gc, x, y, win->bar.l, len);
467+ XDrawString(e->dpy, win->buf.pm, gc, x, y, l->buf, len);
460468 if (len != olen)
461− memcpy(win->bar.l + len - w, rest, w);
469+ memcpy(l->buf + len - w, rest, w);
462470 }
463471 }
464472 }
M window.h View
@@ -49,6 +49,12 @@ typedef struct {
4949 int depth;
5050 } win_env_t;
5151
52+typedef struct {
53+ size_t size;
54+ char *p;
55+ char *buf;
56+} win_bar_t;
57+
5258 typedef struct {
5359 Window xwin;
5460 win_env_t env;
@@ -73,8 +79,8 @@ typedef struct {
7379
7480 struct {
7581 unsigned int h;
76− char l[BAR_L_LEN];
77− char r[BAR_R_LEN];
82+ win_bar_t l;
83+ win_bar_t r;
7884 unsigned long bgcol;
7985 unsigned long fgcol;
8086 } bar;