From 657053f5d04830a226600652d06c2e3392dc95f4 Mon Sep 17 00:00:00 2001 From: Qualys Security Advisory Date: Thu, 1 Jan 1970 00:00:00 +0000 Subject: [PATCH] pgrep: Do not memleak the contents of proc_t. memset()ing task and subtask inside their loops prevents free_acquired() (in readproc() and readtask()) from free()ing their contents (especially cmdline and environ). Our solution is not perfect, because we still memleak the very last cmdline/environ, but select_procs() is called only once, so this is not as bad as it sounds. It would be better to leave subtask in its block and call free_acquired() after the loop, but this function is static (not exported). The only other solution is to use freeproc(), but this means replacing the stack task/subtask with xcalloc()s, thus changing a lot of code in pgrep.c (to pointer accesses). Hence this imperfect solution for now. --- pgrep.c | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/pgrep.c b/pgrep.c index b18df4a7..ce8ccae9 100644 --- a/pgrep.c +++ b/pgrep.c @@ -483,6 +483,7 @@ static struct el * select_procs (int *num) { PROCTAB *ptp; proc_t task; + proc_t subtask; unsigned long long saved_start_time; /* for new/old support */ pid_t saved_pid = 0; /* for new/old support */ int matches = 0; @@ -510,6 +511,7 @@ static struct el * select_procs (int *num) } memset(&task, 0, sizeof (task)); + memset(&subtask, 0, sizeof (subtask)); while(readproc(ptp, &task)) { int match = 1; @@ -615,8 +617,6 @@ static struct el * select_procs (int *num) // argparse time, but a further // control is free if (opt_threads && !i_am_pkill) { - proc_t subtask; - memset(&subtask, 0, sizeof (subtask)); while (readtask(ptp, &task, &subtask)){ // don't add redundand tasks if (task.XXXID == subtask.XXXID) @@ -635,19 +635,9 @@ static struct el * select_procs (int *num) } else { list[matches++].num = subtask.XXXID; } - memset(&subtask, 0, sizeof (subtask)); } } - - - } - - - - - - memset (&task, 0, sizeof (task)); } closeproc (ptp); *num = matches;