Bug#622805: apt: Possible memory leaks and null deferences
Package: apt
Version: 0.8.13.1
Severity: normal
Tags: patch
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
Hi
DACA suggested that there were some issues in APT[1], so I decided
to have a closer look. Please note I suspect that some of these
reported issues are false-positives (Particularly the "Buffer
access out-of-bounds") and I am not sure about the
"object destroyed immediately" errors either.
Nevertheless I deviced a small patch to take care of the memory
leaks and a couple of possible null dereferences[2]. My experience
with APT and C++ is rather limited, so you may want to solve
some of the issues differently.
[1] http://qa.debian.org/daca/cppcheck/sid/apt_0.8.13.2.html
[2] The issues raised by cppcheck here comes from inconsistent
null checks on the variables. E.g. something like
if (log) {
/* use log ...*/
}
log->msg("blah blah blah");
Either the if is redundant or the deference later is a possible
null deference. In my patch I have assumed the latter to the
case always.
- -- Package-specific info:
- -- (no /etc/apt/preferences present) --
- -- (/etc/apt/sources.list present, but not submitted) --
- -- System Information:
Debian Release: wheezy/sid
APT prefers testing
APT policy: (990, 'testing'), (500, 'unstable'), (1, 'experimental')
Architecture: i386 (i686)
Kernel: Linux 2.6.38-2-686 (SMP w/2 CPU cores)
Locale: LANG=en_DK.UTF-8, LC_CTYPE=en_DK.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Versions of packages apt depends on:
ii debian-archive-keyring 2010.08.28 GnuPG archive keys of the Debian a
ii gnupg 1.4.11-3 GNU privacy guard - a free PGP rep
ii libc6 2.11.2-11 Embedded GNU C Library: Shared lib
ii libgcc1 1:4.6.0-2 GCC support library
ii libstdc++6 4.6.0-2 The GNU Standard C++ Library v3
ii zlib1g 1:1.2.3.4.dfsg-3 compression library - runtime
apt recommends no packages.
Versions of packages apt suggests:
pn apt-doc <none> (no description available)
ii aptitude 0.6.3-4 terminal-based package manager (te
ii bzip2 1.0.5-6 high-quality block-sorting file co
ii dpkg-dev 1.15.8.10 Debian package development tools
ii lzma 4.43-14 Compression method of 7z format in
ii python-apt 0.7.100.1 Python interface to libapt-pkg
- -- no debconf information
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
iQIcBAEBCAAGBQJNp0aoAAoJEAVLu599gGRCrK0QAI51s/BPKdsAowFXgYBd5O5M
BqUmgaq3dEJ6B7AbMopMEmosEcciyObHxX2LghrhGIxTaUs+W8/yfQdLayj6wnJo
bNd59jH3nLsiHh1Uo+p5ZcdWe4Nbk3OfsdcbseQfY0iOFcCckivM8piMxcuMDFs2
nSAGkzt1SYEn0m7O3A4z1Z5HijVRwRkEGl07I0UKXh/46y9GN3Y2FdegVqVuBbHn
XnaO2lKllnGrRFtNe7/xa6cY/K7v1K6VwmYP9YNAf8zUoHOfNyHXlz9ToqYwxDqL
uOW2wdmFt8uz0ft4F37IXvanRlUOi2VQshgw/diZZs1RWx2UyoQJ57SpbddkO2tD
M08PTTYBVF4TEDsyWu3jJaV9zu5gaHxMmTiByrvvPgTvOAnnT/+xaPW3tWyOnZmi
2JLdZCEYKey1+9kVUc8SuVkcFRZAkp2FMqA4mHMYy6iF81WY3YyFPTdjRK03rBU2
yYnZQLaPtK4RUFnTKdO/2nA981bSsdeDQ5SMUThSzBf/7yWwqguyVme63SZJBonl
a5CdZefMZDSEM7u1a1eTYsSgj7cW/3UEmj5xk+4D3bNb8Dp9faKFB12ORPGmQ45Y
cqbD3UOBElVMn2Ygyce64G66s42xNzXfbshs8ErH5ADJ/Bnm86GUAao0Q8rAJd5S
VD55U7B8l1N0UkPqODEs
=0NJh
-----END PGP SIGNATURE-----
=== modified file 'apt-inst/dirstream.cc'
--- apt-inst/dirstream.cc 2007-06-14 09:59:13 +0000
+++ apt-inst/dirstream.cc 2011-04-14 18:44:02 +0000
@@ -42,12 +42,20 @@
Itm.Name);
// fchmod deals with umask and fchown sets the ownership
- if (fchmod(iFd,Itm.Mode) != 0)
+ if (fchmod(iFd,Itm.Mode) != 0) {
+ int err = errno;
+ (void)close(iFD);
+ errno = err;
return _error->Errno("fchmod",_("Failed to write file %s"),
Itm.Name);
- if (fchown(iFd,Itm.UID,Itm.GID) != 0 && errno != EPERM)
+ }
+ if (fchown(iFd,Itm.UID,Itm.GID) != 0 && errno != EPERM) {
+ int err = errno;
+ (void)close(iFD);
+ errno = err;
return _error->Errno("fchown",_("Failed to write file %s"),
Itm.Name);
+ }
Fd = iFd;
return true;
}
=== modified file 'apt-pkg/cdrom.cc'
--- apt-pkg/cdrom.cc 2011-03-10 13:42:34 +0000
+++ apt-pkg/cdrom.cc 2011-04-14 18:44:57 +0000
@@ -154,8 +154,12 @@
if (FindPackages(CD + Dir->d_name,List,SList,SigList,TransList,InfoDir,log,Depth+1) == false)
break;
- if (chdir(CD.c_str()) != 0)
+ if (chdir(CD.c_str()) != 0) {
+ int err = errno;
+ (void)closedir(D);
+ errno = err;
return _error->Errno("chdir","Unable to change to %s",CD.c_str());
+ }
};
closedir(D);
@@ -257,8 +261,10 @@
Inodes[I] = Buf.st_ino;
}
- if (_error->PendingError() == true)
+ if (_error->PendingError() == true) {
+ delete[] Inodes;
return false;
+ }
// Look for dups
for (unsigned int I = 0; I != List.size(); I++)
@@ -605,7 +611,9 @@
}
// Mount the new CDROM
- log->Update(_("Mounting CD-ROM...\n"), STEP_MOUNT);
+ if(log) {
+ log->Update(_("Mounting CD-ROM...\n"), STEP_MOUNT);
+ }
if (MountCdrom(CDROM) == false)
return _error->Error("Failed to mount the cdrom.");
}
@@ -749,7 +757,9 @@
log->Update(msg.str());
}
- log->Update(_("Copying package lists..."), STEP_COPY);
+ if(log) {
+ log->Update(_("Copying package lists..."), STEP_COPY);
+ }
// take care of the signatures and copy them if they are ok
// (we do this before PackageCopy as it modifies "List" and "SourceList")
SigVerify SignVerify;
=== modified file 'apt-pkg/contrib/cdromutl.cc'
--- apt-pkg/contrib/cdromutl.cc 2011-03-09 11:15:13 +0000
+++ apt-pkg/contrib/cdromutl.cc 2011-04-14 18:46:30 +0000
@@ -206,8 +206,12 @@
Hash.Add(Dir->d_name);
};
- if (chdir(StartDir.c_str()) != 0)
+ if (chdir(StartDir.c_str()) != 0) {
+ int err = errno;
+ (void)closedir(D);
+ errno = err;
return _error->Errno("chdir",_("Unable to change to %s"),StartDir.c_str());
+ }
closedir(D);
// Some stats from the fsys
@@ -253,8 +257,10 @@
FILE *f=fopen(mount[i], "r");
while ( fgets(buf, sizeof(buf), f) != NULL) {
if (strncmp(buf, devnode, strlen(devnode)) == 0) {
- if(TokSplitString(' ', buf, out, 10))
+ if(TokSplitString(' ', buf, out, 10)) {
+ (void)fclose(f);
return string(out[1]);
+ }
}
}
fclose(f);
=== modified file 'apt-pkg/contrib/error.cc'
--- apt-pkg/contrib/error.cc 2011-02-15 13:04:06 +0000
+++ apt-pkg/contrib/error.cc 2011-04-14 18:46:56 +0000
@@ -107,6 +107,7 @@
msgSize = n + 1;
else
msgSize *= 2;
+ free(S);
return true;
}
@@ -160,6 +161,7 @@
msgSize = n + 1;
else
msgSize *= 2;
+ free(S);
return true;
}
=== modified file 'cmdline/apt-cache.cc'
--- cmdline/apt-cache.cc 2011-03-08 18:32:35 +0000
+++ cmdline/apt-cache.cc 2011-04-14 18:37:51 +0000
@@ -1115,6 +1115,9 @@
}
printf("}\n");
+ delete[] Show;
+ delete[] Flags;
+ delete[] ShapeMap;
return true;
}
/*}}}*/
=== modified file 'cmdline/apt-get.cc'
--- cmdline/apt-get.cc 2011-04-13 09:33:41 +0000
+++ cmdline/apt-get.cc 2011-04-14 18:39:58 +0000
@@ -2386,8 +2386,10 @@
string Src;
pkgSrcRecords::Parser *Last = FindSrc(*I,Recs,SrcRecs,Src,*Cache);
- if (Last == 0)
+ if (Last == 0) {
+ delete[] Dsc;
return _error->Error(_("Unable to find a source package for %s"),Src.c_str());
+ }
string srec = Last->AsStr();
string::size_type pos = srec.find("\nVcs-");
=== modified file 'test/testextract.cc'
--- test/testextract.cc 2004-09-20 17:03:13 +0000
+++ test/testextract.cc 2011-04-14 18:43:19 +0000
@@ -71,9 +71,12 @@
Itm.Type = pkgDirStream::Item::Directory;
int Fd;
- if (Extract.DoItem(Itm,Fd) == false)
+ if (Extract.DoItem(Itm,Fd) == false) {
+ (void)fclose(F);
return false;
+ }
}
+ (void)fclose(F);
}
else
if (Deb.ExtractArchive(Extract) == false)
Reply to: