* 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>
This commit is contained in:
Tomas Hozza 2018-07-30 12:20:27 +02:00 committed by Tim Rühsen
parent 2b2283d3e2
commit b24351183e

View File

@ -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;
}