From 51be550c5c38a2e1ddfc2af50a9fab73ccf78026 Mon Sep 17 00:00:00 2001 From: Julian Andres Klode Date: Mon, 5 Dec 2016 23:01:25 +0100 Subject: 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 Thanks: Jann Horn, Google Project Zero for reporting the issue LP: #1647467 --- apt-pkg/contrib/gpgv.cc | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) (limited to 'apt-pkg') diff --git a/apt-pkg/contrib/gpgv.cc b/apt-pkg/contrib/gpgv.cc index 856d56bc1..7d493fdf6 100644 --- a/apt-pkg/contrib/gpgv.cc +++ b/apt-pkg/contrib/gpgv.cc @@ -287,6 +287,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 * const ContentHeader, FileFd * const SignatureFile) { @@ -302,7 +316,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) @@ -366,6 +381,12 @@ bool SplitClearSignedFile(std::string const &InFile, FileFd * const ContentFile, 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()); -- cgit v1.2.3