Re: Resource leak and possible double-fclose

[Jonathan Nieder <jrnieder@gmail.com>]

Raphael Geissert wrote:
> Hi,
> 
> I found a leak and a possible double-fclosing in some parts of dpkg when 
> checking it with cppcheck. Attached are patches fixing the bugs.

Thanks!

> diff --git a/dpkg-deb/info.c b/dpkg-deb/info.c
> index 9ce7e76..ca9c747 100644
> --- a/dpkg-deb/info.c
> +++ b/dpkg-deb/info.c
> @@ -185,6 +185,8 @@ static void info_list(const char *debar, const char *directory) {
>        putc('\n', stdout);
>    }
>  
> +  fclose(cc);
> +

This seems dangerous: if the control file was missing, this will fclose(NULL),
which might have bad behavior on some platforms.

> @@ -290,8 +290,7 @@ void writecurrentopt() {
>    if (fprintf(cmo,"%s %s\n",coption->meth->name,coption->name) == EOF) {
>      fclose(cmo);
>      ohshite(_("unable to write new option to `%.250s'"),newfile);
> -  }
> -  if (fclose(cmo))
> +  } else if (fclose(cmo))

Isn’t this unnecessary?  ohshite() cannot return.

Here’s a revised patch:

-- %< --
Subject: Fix resource leak in dpkg-deb --info

“dpkg-deb -I foo.deb” leaks the file handle for the package’s
control file.  Found by cppcheck.

Reported-by: Raphael Geissert <atomo64@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 debian/changelog |    4 ++++
 dpkg-deb/info.c  |    1 +
 2 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/debian/changelog b/debian/changelog
index 02d5a43..3a62972 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -63,6 +63,10 @@ dpkg (1.15.6) UNRELEASED; urgency=low
   * Fix error handling, clean up and refactor compression code.
     Thanks to Jonathan Nieder for several of the patches.
 
+  [ Jonathan Nieder ]
+  * Fix a file handle leak in dpkg-deb --info.
+    Thanks to Raphael Geissert for the report and patch.
+
   [ Modestas Vainius ]
   * Implement symbol patterns (Closes: #563752). From now on, it is possible to
     match multiple symbols with a single entry in the symbol file template.
diff --git a/dpkg-deb/info.c b/dpkg-deb/info.c
index 9ce7e76..428f5c4 100644
--- a/dpkg-deb/info.c
+++ b/dpkg-deb/info.c
@@ -183,6 +183,7 @@ static void info_list(const char *debar, const char *directory) {
     }
     if (!lines)
       putc('\n', stdout);
+    fclose(cc);
   }
 
   m_output(stdout, _("<standard output>"));
-- 
1.7.0