Invalid HTTP requests from apt, introduced by DLA 58-1

[Steven McDonald <steven@steven-mcdonald.id.au>]

Hi,

The uploaded package in DLA 58-1 changed apt's http transport in a way
that produces invalid HTTP requests when sending If-Range queries.
Because this upload bundles four changes which haven't been committed
to bzr, I can't be certain which change is responsible, but I suspect
it to be the CVE-2014-6273 fix.

The fix for this is done correctly in the wheezy package (DSA-3031-1);
it is only the backport to squeeze which is buggy, which is why I am
posting this to -lts.

The problematic part of the DLA 58-1 diff is:

-------------------------- 8< --------------------------
--- apt-0.8.10.3+squeeze3/methods/http.cc
+++ apt-0.8.10.3+squeeze5/methods/http.cc
@@ -716,15 +714,17 @@
    if (stat(Itm->DestFile.c_str(),&SBuf) >= 0 && SBuf.st_size > 0)
    {
       // In this case we send an if-range query with a range header
-      sprintf(Buf,"Range: bytes=%li-\r\nIf-Range: %s\r\n",(long)SBuf.st_size - 1,
+      std::string Tmp;
+      strprintf(Tmp,"Range: bytes=%li-\r\nIf-Range: %s\r\n",(long)SBuf.st_size - 1,
 	      TimeRFC1123(SBuf.st_mtime).c_str());
+      Buf += Tmp;
       Req += Buf;
    }
    else
-------------------------- >8 --------------------------

By appending Tmp to Buf instead of overwriting it, the entire
accumulated request so far gets appended to itself instead of just
these two headers, resulting in a request that looks like this:

-------------------------- 8< --------------------------
GET /some/url HTTP/1.1
Host: host.domain
GET /some/url HTTP/1.1
Host: host.domain
Range: ...
-------------------------- >8 --------------------------

As far as I can tell, there is no reason for Tmp to exist at all. The
fix for this in wheezy only changes sprintf to strprintf, leaving Buf
as its first argument. My proposed fix would be the following:

-------------------------- 8< --------------------------
--- apt-0.8.10.3+squeeze5.orig/methods/http.cc
+++ apt-0.8.10.3+squeeze5/methods/http.cc
@@ -714,10 +714,8 @@
    if (stat(Itm->DestFile.c_str(),&SBuf) >= 0 && SBuf.st_size > 0)
    {
       // In this case we send an if-range query with a range header
-      std::string Tmp;
-      strprintf(Tmp,"Range: bytes=%li-\r\nIf-Range: %s\r\n",(long)SBuf.st_size - 1,
+      strprintf(Buf,"Range: bytes=%li-\r\nIf-Range: %s\r\n",(long)SBuf.st_size - 1,
 	      TimeRFC1123(SBuf.st_mtime).c_str());
-      Buf += Tmp;
       Req += Buf;
    }
    else
-------------------------- >8 --------------------------

Please let me know if you need any further information. I am subscribed
to the list, so there is no need to CC me.

Thanks,
Steven.