[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

[PATCH v2] Fix resource leak in dpkg-deb --info

“dpkg-deb -I foo.deb” leaks the file handle for the package’s
control file.  Check for read errors and close the file before
it falls out of scope.

Found by cppcheck.

Reported-by: Raphael Geissert <atomo64@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Raphael Geissert wrote:

> Right, should have been more careful. This is a false positive.

No problem --- finding the problems and writing a first patch
is half the work already.  Consider this a wishlist bug for the
static analyzers: if they would output their suggestions in the
form of a patch, I would find them much more usable...

Looking over the patch again, it seems I forgot to check for
errors before closing control.  Here’s a revised patch.

Thanks again,

 debian/changelog |    4 ++++
 dpkg-deb/info.c  |    4 ++++
 2 files changed, 8 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..6f58dc1 100644
--- a/dpkg-deb/info.c
+++ b/dpkg-deb/info.c
@@ -183,6 +183,10 @@ static void info_list(const char *debar, const char *directory) {
     if (!lines)
       putc('\n', stdout);
+    if (ferror(cc))
+      ohshite(_("failed to read `%.255s' (in `%.255s')"),
+              "control", directory);
+    fclose(cc);
   m_output(stdout, _("<standard output>"));

Reply to: