Re: ftp: leak in progressmeter()

[Alexander Bluhm]

On Sun, Sep 03, 2017 at 03:19:15PM -0500, Scott Cheloha wrote:
> Feedback?

It looks like progressmeter() can be called with flag -1 consecutively
due to some error path.  So better free the title.  It cannot hurt.

OK bluhm@

> Index: usr.bin/ftp/util.c
> ===
> RCS file: /cvs/src/usr.bin/ftp/util.c,v
> retrieving revision 1.84
> diff -u -p -r1.84 util.c
> --- usr.bin/ftp/util.c21 Jan 2017 08:33:07 -  1.84
> +++ usr.bin/ftp/util.c3 Sep 2017 19:52:26 -
> @@ -784,8 +784,10 @@ progressmeter(int flag, const char *file
>   ratio = MINIMUM(ratio, 100);
>   if (!verbose && flag == -1) {
>   filename = basename(filename);
> - if (filename != NULL)
> + if (filename != NULL) {
> + free(title);
>   title = strdup(filename);
> + }
>   }
>  
>   buf[0] = 0;

ftp: leak in progressmeter()

[Scott Cheloha]

Hi,

In ftp(1), when displaying the progress meter while not running in
verbose mode, if a transfer is aborted and a subsequent transfer is
started, we leak title.

Given that we only ever print the first 25 bytes of the remote file
name we can definitely display the title without going to strdup(3),
but the code in progressmeter() is really hairy so I think this will
do for now.

Anyway, title gets nullified (NULL'd?) after it is freed in the
successful case, and it is non-global and static and initialized to
NULL, so adding a free before we allocate it plugs the leak.

Feedback?

--
Scott Cheloha

Index: usr.bin/ftp/util.c
===
RCS file: /cvs/src/usr.bin/ftp/util.c,v
retrieving revision 1.84
diff -u -p -r1.84 util.c
--- usr.bin/ftp/util.c  21 Jan 2017 08:33:07 -  1.84
+++ usr.bin/ftp/util.c  3 Sep 2017 19:52:26 -
@@ -784,8 +784,10 @@ progressmeter(int flag, const char *file
ratio = MINIMUM(ratio, 100);
if (!verbose && flag == -1) {
filename = basename(filename);
-   if (filename != NULL)
+   if (filename != NULL) {
+   free(title);
title = strdup(filename);
+   }
}
 
buf[0] = 0;