diff options
author | Julian Andres Klode <jak@debian.org> | 2016-12-05 23:01:25 +0100 |
---|---|---|
committer | Julian Andres Klode <jak@debian.org> | 2016-12-08 15:31:19 +0100 |
commit | 538b04f7b31ad1e2fb04804574614762001be136 (patch) | |
tree | 9973cf3f9ed3313c487cf35e2a7569a48ca794a1 | |
parent | 66b687801a7bad997f2017b423f484604e9c0787 (diff) | |
download | apt-538b04f7b31ad1e2fb04804574614762001be136.tar.gz |
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)
(cherry picked from commit 4ef9e0837ce139b398299431ae2294882f531d8e)
(cherry picked from commit 0bbbabb1b961b3b6541e7fdc8061fe6f282eafad)
-rw-r--r-- | apt-pkg/contrib/gpgv.cc | 23 |
1 files changed, 22 insertions, 1 deletions
diff --git a/apt-pkg/contrib/gpgv.cc b/apt-pkg/contrib/gpgv.cc index f24dd9640..0d74050cb 100644 --- a/apt-pkg/contrib/gpgv.cc +++ b/apt-pkg/contrib/gpgv.cc @@ -247,6 +247,20 @@ void ExecGPGV(std::string const &File, std::string const &FileGPG, } /*}}}*/ // 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) { @@ -262,7 +276,8 @@ bool SplitClearSignedFile(std::string const &InFile, FileFd * const ContentFile, 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) @@ -324,6 +339,12 @@ bool SplitClearSignedFile(std::string const &InFile, FileFd * const ContentFile, } fclose(in); + // 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()); |