]> git.saurik.com Git - apt.git/commitdiff
ensure that pkgTagFile isn't writing past Buffer length
authorDavid Kalnischkies <kalnischkies@gmail.com>
Thu, 15 Aug 2013 14:49:51 +0000 (16:49 +0200)
committerDavid Kalnischkies <kalnischkies@gmail.com>
Thu, 15 Aug 2013 15:39:24 +0000 (17:39 +0200)
In 91c4cc14d3654636edf997d23852f05ad3de4853 I removed the +256 from
the pkgTagFile call parsing Release files as I couldn't find a
mentioning of a reason for why and it was marked as XXX which suggested
that at least someone else was suspicious.

It turns out that it is indeed "documented", it just didn't found it at
first but the changelog of apt 0.6.6 (29. Dec 2003) mentions:
  * Restore the ugly hack I removed from indexRecords::Load which set the
    pkgTagFile buffer size to (file size)+256.  This is concealing a bug,
    but I can't fix it right now.  This should fix the segfaults that
    folks are seeing with 0.6.[45].

The bug it is "hiding" is that if pkgTagFile works with a file which doesn't
end in a double newline it will be adding it without checking if the Buffer
is big enough to store them. Its also not a good idea to let the End
pointer be past the end of our space, even if we don't access the data.

Closes: 719629
apt-pkg/tagfile.cc
apt-pkg/tagfile.h

index 1c79ee74ffaecc559f2a2daad8a4f63d30bdcce2..aa0f1eee8a15ad5b5b295026178ea476db1b4804 100644 (file)
@@ -50,6 +50,11 @@ public:
 /* */
 pkgTagFile::pkgTagFile(FileFd *pFd,unsigned long long Size)
 {
+   /* The size is increased by 4 because if we start with the Size of the
+      filename we need to try to read 1 char more to see an EOF faster, 1
+      char the end-pointer can be on and maybe 2 newlines need to be added
+      to the end of the file -> 4 extra chars */
+   Size += 4;
    d = new pkgTagFilePrivate(pFd, Size);
 
    if (d->Fd.IsOpen() == false)
@@ -89,17 +94,21 @@ unsigned long pkgTagFile::Offset()
  */
 bool pkgTagFile::Resize()
 {
-   char *tmp;
-   unsigned long long EndSize = d->End - d->Start;
-
    // fail is the buffer grows too big
    if(d->Size > 1024*1024+1)
       return false;
 
+   return Resize(d->Size * 2);
+}
+bool pkgTagFile::Resize(unsigned long long const newSize)
+{
+   unsigned long long const EndSize = d->End - d->Start;
+   char *tmp;
+
    // get new buffer and use it
-   tmp = new char[2*d->Size];
+   tmp = new char[newSize];
    memcpy(tmp, d->Buffer, d->Size);
-   d->Size = d->Size*2;
+   d->Size = newSize;
    delete [] d->Buffer;
    d->Buffer = tmp;
 
@@ -152,9 +161,10 @@ bool pkgTagFile::Fill()
    if (d->Done == false)
    {
       // See if only a bit of the file is left
-      if (d->Fd.Read(d->End, d->Size - (d->End - d->Buffer),&Actual) == false)
+      unsigned long long const dataSize = d->Size - ((d->End - d->Buffer) + 1);
+      if (d->Fd.Read(d->End, dataSize, &Actual) == false)
         return false;
-      if (Actual != d->Size - (d->End - d->Buffer))
+      if (Actual != dataSize || d->Fd.Eof() == true)
         d->Done = true;
       d->End += Actual;
    }
@@ -171,8 +181,13 @@ bool pkgTagFile::Fill()
       for (const char *E = d->End - 1; E - d->End < 6 && (*E == '\n' || *E == '\r'); E--)
         if (*E == '\n')
            LineCount++;
-      for (; LineCount < 2; LineCount++)
-        *d->End++ = '\n';
+      if (LineCount < 2)
+      {
+        if ((unsigned)(d->End - d->Buffer) >= d->Size)
+           Resize(d->Size + 3);
+        for (; LineCount < 2; LineCount++)
+           *d->End++ = '\n';
+      }
       
       return true;
    }
index fedd72701528856cc930e5a8035fd22ae1895cf7..66c56799dd42c0408683e2cd0f074f8e9bba1679 100644 (file)
@@ -95,6 +95,7 @@ class pkgTagFile
 
    bool Fill();
    bool Resize();
+   bool Resize(unsigned long long const newSize);
 
    public: