SECURITY UPDATE: gpgv: Check for errors when splitting files (CVE-2016-1252)

This fixes a security issue where signatures of the
InRelease files could be circumvented in a man-in-the-middle
attack, giving attackers the ability to serve any packages
they want to a system, in turn giving them root access.

It turns out that getline() may not only return EINVAL
as stated in the documentation - it might also return
in case of an error when allocating memory.

This fix not only adds a check that reading worked
correctly, it also implicitly checks that all writes
worked by reporting any other error that occurred inside
the loop and was logged by apt.

Affected: >= 0.9.8
Reported-By: Jann Horn <jannh@google.com>
Thanks: Jann Horn, Google Project Zero for reporting the issue
LP: #1647467
(cherry picked from commit 51be550c5c38a2e1ddfc2af50a9fab73ccf78026)

diff --git a/apt-pkg/contrib/gpgv.cc b/apt-pkg/contrib/gpgv.cc
index 941f901e8ffedd832bf4cff2a47b73a2bca283d5..b68e5f17219800d8beb062ed6bee8b7ae286e61d 100644 (file)
--- a/apt-pkg/contrib/gpgv.cc
+++ b/apt-pkg/contrib/gpgv.cc
@@ -244,6 +244,20 @@ void ExecGPGV(std::string const &File, std::string const &FileGPG,
 }
                                                                        /*}}}*/
 // SplitClearSignedFile - split message into data/signature            /*{{{*/
 }
                                                                        /*}}}*/
 // SplitClearSignedFile - split message into data/signature            /*{{{*/

+static int GetLineErrno(char **lineptr, size_t *n, FILE *stream, std::string const &InFile)
+{
+   int result;
+
+   errno = 0;
+   result = getline(lineptr, n, stream);
+   if (errno != 0)
+   {
+      _error->Errno("getline", "Could not read from %s", InFile.c_str());
+      return -1;
+   }
+
+   return result;
+}

 bool SplitClearSignedFile(std::string const &InFile, FileFd * const ContentFile,
       std::vector<std::string> * const ContentHeader, FileFd * const SignatureFile)
 {
 bool SplitClearSignedFile(std::string const &InFile, FileFd * const ContentFile,
       std::vector<std::string> * const ContentHeader, FileFd * const SignatureFile)
 {


@@ -259,7 +273,8 @@ bool SplitClearSignedFile(std::string const &InFile, FileFd * const ContentFile,
 
    char *buf = NULL;
    size_t buf_size = 0;
 
    char *buf = NULL;
    size_t buf_size = 0;

-   while (getline(&buf, &buf_size, in) != -1)
+   _error->PushToStack();
+   while (GetLineErrno(&buf, &buf_size, in, InFile) != -1)

    {
       _strrstrip(buf);
       if (found_message_start == false)
    {
       _strrstrip(buf);
       if (found_message_start == false)


@@ -323,6 +338,12 @@ bool SplitClearSignedFile(std::string const &InFile, FileFd * const ContentFile,
    if (buf != NULL)
       free(buf);
 
    if (buf != NULL)
       free(buf);
 

+   // An error occured during reading - propagate it up
+   bool const hasErrored = _error->PendingError();
+   _error->MergeWithStack();
+   if (hasErrored)
+      return false;
+

    if (found_signature == true)
       return _error->Error("Signature in file %s wasn't closed", InFile.c_str());
 
    if (found_signature == true)
       return _error->Error("Signature in file %s wasn't closed", InFile.c_str());