From b24351183ec574f81c729cbb3286aceaee3f03c8 Mon Sep 17 00:00:00 2001
From: Tomas Hozza <thozza@redhat.com>
Date: Mon, 30 Jul 2018 12:20:27 +0200
Subject: [PATCH] * src/ftp.c (getftp): Fix RESOURCE LEAK found by Coverity
Error: RESOURCE_LEAK (CWE-772):
wget-1.19.5/src/ftp.c:1493: alloc_fn: Storage is returned from allocation function "fopen".
wget-1.19.5/src/ftp.c:1493: var_assign: Assigning: "fp" = storage returned from "fopen(con->target, "wb")".
wget-1.19.5/src/ftp.c:1811: leaked_storage: Variable "fp" going out of scope leaks the storage it points to.
\# 1809| if (fp && !output_stream)
\# 1810| fclose (fp);
\# 1811|-> return err;
\# 1812| }
\# 1813|
It can happen, that "if (!output_stream || con->cmd & DO_LIST)" on line #1398 can be true, even though "output_stream != NULL". In this case a new file is opened to "fp". Later it may happen in the FTPS branch, that some error will occure and code will jump to label "exit_error". In "exit_error", the "fp" is closed only if "output_stream == NULL". However this may not be true as described earlier and "fp" leaks.
On line #1588, there is the following conditional free of "fp":
/* Close the local file. */
if (!output_stream || con->cmd & DO_LIST)
fclose (fp);
Therefore the conditional at the end of the function after "exit_error" label should be modified to:
if (fp && (!output_stream || con->cmd & DO_LIST))
fclose (fp);
This will ensure that "fp" does not leak in any case it sould be opened.
Signed-off-by: Tomas Hozza <thozza@redhat.com>
---
src/ftp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/ftp.c b/src/ftp.c
index 69148936..daaae939 100644
--- a/src/ftp.c
+++ b/src/ftp.c
@@ -1806,7 +1806,7 @@ Error in server response, closing control connection.\n"));
exit_error:
/* If fp is a regular file, close and try to remove it */
- if (fp && !output_stream)
+ if (fp && (!output_stream || con->cmd & DO_LIST))
fclose (fp);
return err;
}