Bug#319142: apt: attempts to install corrupted packages

[Michael Vogt <michael.vogt@ubuntu.com>]

On Wed, Jul 20, 2005 at 09:38:08AM +0200, Goswin von Brederlow wrote:
> Andrew Suffield <asuffield@debian.org> writes:
> 
> > Package: apt
> > Severity: important

Thanks for your bugreport and sorry for my late reply. 

> > asuffield@cyclone:~$ apt-cache show xfonts-scalable
> > Package: xfonts-scalable
> > ...
> > Version: 6.8.2.dfsg.1-3
> > ...
> > MD5sum: 0e9e786a6220993510e2b9cfdbc65ee1
> >
> > Preparing to replace xfonts-scalable 4.3.0.dfsg.1-14 (using .../xfonts-scalable_6.8.2.dfsg.1-3_all.deb) ...
> > Unpacking replacement xfonts-scalable ...
> > dpkg: error processing /var/cache/apt/archives/xfonts-scalable_6.8.2.dfsg.1-3_all.deb (--unpack):
> >  corrupted filesystem tarfile - corrupted package archive: Success
> > dpkg-deb: subprocess paste killed by signal (Broken pipe)
> >
> > asuffield@cyclone:~$ md5sum /var/cache/apt/archives/xfonts-scalable_6.8.2.dfsg.1-3_all.deb
> > a525d80fb0df950f4e9b0e3141c63d0c  /var/cache/apt/archives/xfonts-scalable_6.8.2.dfsg.1-3_all.deb
> >
> > Not only is this broken and annoying, it indicates that the security
> > checking code is completely non-functional.

I can't reproduce the problem here for http/ftp/ssh/rsh. Downloading
certainly does md5sum checks and packages that do not match the md5sum
will fail. I was able to reproduce it for file uries.

> I can confirm this bus at least in part (see below).
> 
> Andrew: What does your sources.list look like? Do you have a local
> mirror with the corrupted file (file or copy url)?

Andrew: could you please attach your sources.list?
 
> I did some testing, purposefully corrupting a deb, to see what gets
> checked and what not (apt 0.6.38):
[..]
> So I can confirm the bug for file (and copy) urls. Is that because
> apt-get considers them local and they don't go through
> /var/lib/apt/cache/partial?

I reproduced the problem here for file and cdrom urls. 

The attached patch (also in
michael.vogt@ubuntu.com--2005/apt--fixes--0--patch-14) added md5sum
checking for file and cdrom methods. Support in copy is not needed
because it is only used internally by the other methods.

 
Cheers,
 Michael

-- 
Linux is not The Answer. Yes is the answer. Linux is The Question. - Neo

* looking for apt@packages.debian.org/apt--main--0--patch-100 to compare with
* comparing to apt@packages.debian.org/apt--main--0--patch-100: .... done.

* modified files

--- orig/apt-pkg/acquire-item.cc
+++ mod/apt-pkg/acquire-item.cc
@@ -898,7 +898,8 @@
       {
 	 Status = StatError;
 	 ErrorText = _("MD5Sum mismatch");
-	 Rename(DestFile,DestFile + ".FAILED");
+	 if(FileExists(DestFile))
+	    Rename(DestFile,DestFile + ".FAILED");
 	 return;
       }
    }


--- orig/cmdline/apt-get.cc
+++ mod/cmdline/apt-get.cc
@@ -1657,7 +1657,7 @@
    // See if we need to prompt
    if (Cache->InstCount() == ExpectedInst && Cache->DelCount() == 0)
       return InstallPackages(Cache,false,false);
-   
+
    return InstallPackages(Cache,false);   
 }
 									/*}}}*/


--- orig/methods/cdrom.cc
+++ mod/methods/cdrom.cc
@@ -13,6 +13,7 @@
 #include <apt-pkg/error.h>
 #include <apt-pkg/configuration.h>
 #include <apt-pkg/fileutl.h>
+#include <apt-pkg/hashes.h>
 
 #include <sys/stat.h>
 #include <unistd.h>
@@ -180,6 +181,12 @@
       CurrentID = NewID;
    Res.LastModified = Buf.st_mtime;
    Res.Size = Buf.st_size;
+
+   Hashes Hash;
+   FileFd Fd(Res.Filename, FileFd::ReadOnly);
+   Hash.AddFD(Fd.Fd(), Fd.Size());
+   Res.TakeHashes(Hash);
+
    URIDone(Res);
    return true;
 }


--- orig/methods/file.cc
+++ mod/methods/file.cc
@@ -15,6 +15,8 @@
 // Include Files							/*{{{*/
 #include <apt-pkg/acquire-method.h>
 #include <apt-pkg/error.h>
+#include <apt-pkg/hashes.h>
+#include <apt-pkg/fileutl.h>
 
 #include <sys/stat.h>
 #include <unistd.h>
@@ -75,7 +77,11 @@
    
    if (Res.Filename.empty() == true)
       return _error->Error(_("File not found"));
-   
+
+   Hashes Hash;
+   FileFd Fd(Res.Filename, FileFd::ReadOnly);
+   Hash.AddFD(Fd.Fd(), Fd.Size());
+   Res.TakeHashes(Hash);
    URIDone(Res);
    return true;
 }