From f50c812648e496769d9cd55186091b51a80fca0e Mon Sep 17 00:00:00 2001 From: Tomas Hozza Date: Mon, 30 Jul 2018 12:20:27 +0200 Subject: [PATCH 1/6] Fix RESOURCE LEAK in ftp.c 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 --- 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; } -- 2.17.1