summaryrefslogtreecommitdiff
path: root/apt-pkg/contrib/gpgv.cc
diff options
context:
space:
mode:
authorJulian Andres Klode <jak@debian.org>2016-12-05 23:01:25 +0100
committerJulian Andres Klode <jak@debian.org>2016-12-08 15:27:57 +0100
commit0bbbabb1b961b3b6541e7fdc8061fe6f282eafad (patch)
tree4cf88748dbcf2c210b60ec1631372f7ba2622290 /apt-pkg/contrib/gpgv.cc
parent2234ff7c2b143046fd196f544ca4baccc7e2b2ec (diff)
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)
Diffstat (limited to 'apt-pkg/contrib/gpgv.cc')
-rw-r--r--apt-pkg/contrib/gpgv.cc23
1 files changed, 22 insertions, 1 deletions
diff --git a/apt-pkg/contrib/gpgv.cc b/apt-pkg/contrib/gpgv.cc
index 4a2f34043..7e7b3ac80 100644
--- a/apt-pkg/contrib/gpgv.cc
+++ b/apt-pkg/contrib/gpgv.cc
@@ -235,6 +235,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)
{
@@ -250,7 +264,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)
@@ -314,6 +329,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());