Don't grow logevent buf indefinitely

The PuTTY GUIs (Unix and Windows) maintain an in-memory event log
for display to users as they request.  This uses ints for tracking
eventlog size, which is subject to memory exhaustion and (given
enough heap space) overflow attacks by servers (via, e.g., constant
rekeying).

Also a bounded log is more user-friendly.  It is rare to want more
than the initial logging and the logging from a few recent rekey
events.

The Windows fix has been tested using Dr. Memory as a valgrind
substitute.  No errors corresponding to the affected code showed up.
The Dr. Memory results.txt was split into a file per-error and then

    grep Error $(grep -l windlg *)|cut -d: -f3-|sort |uniq -c

was used to compare.  Differences arose from different usage of the GUI,
but no error could be traced to the code modified in this commit.

The Unix fix has been tested using valgrind.  We don't destroy the
eventlog_stuff eventlog arrays, so we can't be entirely sure that we
don't leak more than we did before, but from code inspection it looks
like we don't (and anyways, if we leaked as much as before, just without
the integer overflow, well, that's still an improvement).
This commit is contained in:
Nico Williams 2013-07-22 19:09:04 -04:00 коммит произвёл Simon Tatham
Родитель b26bd60df9
Коммит 3447047594
2 изменённых файлов: 97 добавлений и 33 удалений

Просмотреть файл

@ -3770,14 +3770,18 @@ void about_box(void *window)
gtk_window_set_focus(GTK_WINDOW(aboutbox), NULL);
}
#define LOGEVENT_INITIAL_MAX 128
#define LOGEVENT_CIRCULAR_MAX 128
struct eventlog_stuff {
GtkWidget *parentwin, *window;
struct controlbox *eventbox;
struct Shortcuts scs;
struct dlgparam dp;
union control *listctrl;
char **events;
int nevents, negsize;
char **events_initial;
char **events_circular;
int ninitial, ncircular, circular_first;
char *seldata;
int sellen;
int ignore_selchange;
@ -3809,8 +3813,11 @@ static void eventlog_list_handler(union control *ctrl, void *dlg,
dlg_update_start(ctrl, dlg);
dlg_listbox_clear(ctrl, dlg);
for (i = 0; i < es->nevents; i++) {
dlg_listbox_add(ctrl, dlg, es->events[i]);
for (i = 0; i < es->ninitial; i++) {
dlg_listbox_add(ctrl, dlg, es->events_initial[i]);
}
for (i = 0; i < es->ncircular; i++) {
dlg_listbox_add(ctrl, dlg, es->events_circular[(es->circular_first + i) % LOGEVENT_CIRCULAR_MAX]);
}
dlg_update_done(ctrl, dlg);
} else if (event == EVENT_SELCHANGE) {
@ -3832,16 +3839,31 @@ static void eventlog_list_handler(union control *ctrl, void *dlg,
sfree(es->seldata);
es->seldata = NULL;
es->sellen = 0;
for (i = 0; i < es->nevents; i++) {
for (i = 0; i < es->ninitial; i++) {
if (dlg_listbox_issel(ctrl, dlg, i)) {
int extralen = strlen(es->events[i]);
int extralen = strlen(es->events_initial[i]);
if (es->sellen + extralen + 2 > selsize) {
selsize = es->sellen + extralen + 512;
es->seldata = sresize(es->seldata, selsize, char);
}
strcpy(es->seldata + es->sellen, es->events[i]);
strcpy(es->seldata + es->sellen, es->events_initial[i]);
es->sellen += extralen;
es->seldata[es->sellen++] = '\n';
}
}
for (i = 0; i < es->ncircular; i++) {
if (dlg_listbox_issel(ctrl, dlg, es->ninitial + i)) {
int j = (es->circular_first + i) % LOGEVENT_CIRCULAR_MAX;
int extralen = strlen(es->events_circular[j]);
if (es->sellen + extralen + 2 > selsize) {
selsize = es->sellen + extralen + 512;
es->seldata = sresize(es->seldata, selsize, char);
}
strcpy(es->seldata + es->sellen, es->events_circular[j]);
es->sellen += extralen;
es->seldata[es->sellen++] = '\n';
}
@ -3990,25 +4012,42 @@ void *eventlogstuff_new(void)
void logevent_dlg(void *estuff, const char *string)
{
struct eventlog_stuff *es = (struct eventlog_stuff *)estuff;
char timebuf[40];
struct tm tm;
char **location;
size_t i;
if (es->nevents >= es->negsize) {
es->negsize += 64;
es->events = sresize(es->events, es->negsize, char *);
if (es->ninitial == 0) {
es->events_initial = sresize(es->events_initial, LOGEVENT_INITIAL_MAX, char *);
for (i = 0; i < LOGEVENT_INITIAL_MAX; i++)
es->events_initial[i] = NULL;
es->events_circular = sresize(es->events_circular, LOGEVENT_CIRCULAR_MAX, char *);
for (i = 0; i < LOGEVENT_CIRCULAR_MAX; i++)
es->events_circular[i] = NULL;
}
if (es->ninitial < LOGEVENT_INITIAL_MAX)
location = &es->events_initial[es->ninitial];
else
location = &es->events_circular[(es->circular_first + es->ncircular) % LOGEVENT_CIRCULAR_MAX];
tm=ltime();
strftime(timebuf, sizeof(timebuf), "%Y-%m-%d %H:%M:%S\t", &tm);
es->events[es->nevents] = snewn(strlen(timebuf) + strlen(string) + 1, char);
strcpy(es->events[es->nevents], timebuf);
strcat(es->events[es->nevents], string);
sfree(*location);
*location = dupcat(timebuf, string, NULL);
if (es->window) {
dlg_listbox_add(es->listctrl, &es->dp, es->events[es->nevents]);
dlg_listbox_add(es->listctrl, &es->dp, *location);
}
if (es->ninitial < LOGEVENT_INITIAL_MAX) {
es->ninitial++;
} else if (es->ncircular < LOGEVENT_CIRCULAR_MAX) {
es->ncircular++;
} else if (es->ncircular == LOGEVENT_CIRCULAR_MAX) {
es->circular_first = (es->circular_first + 1) % LOGEVENT_CIRCULAR_MAX;
sfree(es->events_circular[es->circular_first]);
es->events_circular[es->circular_first] = dupstr("..");
}
es->nevents++;
}
int askappend(void *frontend, Filename *filename,

Просмотреть файл

@ -42,8 +42,12 @@ static struct controlbox *ctrlbox;
static struct winctrls ctrls_base, ctrls_panel;
static struct dlgparam dp;
static char **events = NULL;
static int nevents = 0, negsize = 0;
#define LOGEVENT_INITIAL_MAX 128
#define LOGEVENT_CIRCULAR_MAX 128
static char *events_initial[LOGEVENT_INITIAL_MAX];
static char *events_circular[LOGEVENT_CIRCULAR_MAX];
static int ninitial = 0, ncircular = 0, circular_first = 0;
extern Conf *conf; /* defined in window.c */
@ -67,6 +71,15 @@ void force_normal(HWND hwnd)
recurse = 0;
}
static char *getevent(int i)
{
if (i < ninitial)
return events_initial[i];
if ((i -= ninitial) < ncircular)
return events_circular[(circular_first + i) % LOGEVENT_CIRCULAR_MAX];
return NULL;
}
static INT_PTR CALLBACK LogProc(HWND hwnd, UINT msg,
WPARAM wParam, LPARAM lParam)
{
@ -84,9 +97,12 @@ static INT_PTR CALLBACK LogProc(HWND hwnd, UINT msg,
SendDlgItemMessage(hwnd, IDN_LIST, LB_SETTABSTOPS, 2,
(LPARAM) tabs);
}
for (i = 0; i < nevents; i++)
for (i = 0; i < ninitial; i++)
SendDlgItemMessage(hwnd, IDN_LIST, LB_ADDSTRING,
0, (LPARAM) events[i]);
0, (LPARAM) events_initial[i]);
for (i = 0; i < ncircular; i++)
SendDlgItemMessage(hwnd, IDN_LIST, LB_ADDSTRING,
0, (LPARAM) events_circular[(circular_first + i) % LOGEVENT_CIRCULAR_MAX]);
return 1;
case WM_COMMAND:
switch (LOWORD(wParam)) {
@ -127,13 +143,13 @@ static INT_PTR CALLBACK LogProc(HWND hwnd, UINT msg,
size = 0;
for (i = 0; i < count; i++)
size +=
strlen(events[selitems[i]]) + sizeof(sel_nl);
strlen(getevent(selitems[i])) + sizeof(sel_nl);
clipdata = snewn(size, char);
if (clipdata) {
char *p = clipdata;
for (i = 0; i < count; i++) {
char *q = events[selitems[i]];
char *q = getevent(selitems[i]);
int qlen = strlen(q);
memcpy(p, q, qlen);
p += qlen;
@ -145,7 +161,7 @@ static INT_PTR CALLBACK LogProc(HWND hwnd, UINT msg,
}
sfree(selitems);
for (i = 0; i < nevents; i++)
for (i = 0; i < (ninitial + ncircular); i++)
SendDlgItemMessage(hwnd, IDN_LIST, LB_SETSEL,
FALSE, i);
}
@ -748,29 +764,38 @@ int do_reconfig(HWND hwnd, int protcfginfo)
void logevent(void *frontend, const char *string)
{
char timebuf[40];
char **location;
struct tm tm;
log_eventlog(logctx, string);
if (nevents >= negsize) {
negsize += 64;
events = sresize(events, negsize, char *);
}
tm=ltime();
strftime(timebuf, sizeof(timebuf), "%Y-%m-%d %H:%M:%S\t", &tm);
events[nevents] = snewn(strlen(timebuf) + strlen(string) + 1, char);
strcpy(events[nevents], timebuf);
strcat(events[nevents], string);
if (ninitial < LOGEVENT_INITIAL_MAX)
location = &events_initial[ninitial];
else
location = &events_circular[(circular_first + ncircular) % LOGEVENT_CIRCULAR_MAX];
if (*location)
sfree(*location);
*location = dupcat(timebuf, string, NULL);
if (logbox) {
int count;
SendDlgItemMessage(logbox, IDN_LIST, LB_ADDSTRING,
0, (LPARAM) events[nevents]);
0, (LPARAM) *location);
count = SendDlgItemMessage(logbox, IDN_LIST, LB_GETCOUNT, 0, 0);
SendDlgItemMessage(logbox, IDN_LIST, LB_SETTOPINDEX, count - 1, 0);
}
nevents++;
if (ninitial < LOGEVENT_INITIAL_MAX) {
ninitial++;
} else if (ncircular < LOGEVENT_CIRCULAR_MAX) {
ncircular++;
} else if (ncircular == LOGEVENT_CIRCULAR_MAX) {
circular_first = (circular_first + 1) % LOGEVENT_CIRCULAR_MAX;
sfree(events_circular[circular_first]);
events_circular[circular_first] = dupstr("..");
}
}
void showeventlog(HWND hwnd)