Request 2797 (accepted)

Overview

Request 2797 (accepted)

Fix memory-safety bugs

Created by sicherha almost 4 years ago
In state accepted
Open review for Admin
Open review for vanmeeuwen

Submit libkolab-old

Submit package home:sicherha...hes:Kolab:16 / libkolab-old to package Kolab:16 / libkolab-old

libkolab.spec Changed

0001-Fix-dangling-pointers.patch Added

@@ -0,0 +1,64 @@
+From 4bda8a1cb3befa5dc4b03c6590640053afb74ef5 Mon Sep 17 00:00:00 2001
+From: Christoph Erhardt <kolab@sicherha.de>
+Date: Tue, 25 May 2021 08:57:22 +0200
+Subject: [PATCH 1/2] Fix dangling pointers
+
+std::vector gives zero guarantees that pointers to its elements remain
+valid when the vector's size changes. In particular, pushing new
+elements into the vector may trigger reallocation of the underlying heap
+area.
+
+Consequently, Event::delegate() needs to ensure that any modifications
+to the d->attendees vector are performed before pointers to its elements
+are taken and collected.
+
+Found with Valgrind.
+
+
+Reviewers: mollekopf
+
+Reviewed By: mollekopf
+
+Subscribers: mollekopf
+
+Differential Revision: https://git.kolab.org/D2548
+---
+ calendaring/event.cpp | 17 +++++++++++------
+ 1 file changed, 11 insertions(+), 6 deletions(-)
+
+diff --git a/calendaring/event.cpp b/calendaring/event.cpp
+index bcdf833..ea9ead1 100644
+--- a/calendaring/event.cpp
++++ b/calendaring/event.cpp
+@@ -76,17 +76,22 @@ bool contains(const Kolab::ContactReference &delegatorRef, const std::vector <Ko
+ void Event::delegate(const std::vector< Attendee >& delegators, const std::vector< Attendee >& delegatees)
+ {
+ 
+-    //First build a list of attendee references, and insert any missing attendees
+-    std::vector<Kolab::Attendee*> delegateesRef;
++    //First insert any missing attendees
+     foreach(const Attendee &a, delegatees) {
+-        if (Attendee *attendee = getAttendee(a.contact())) {
+-            delegateesRef.push_back(attendee);
+-        } else {
++        if (!getAttendee(a.contact())) {
+             d->attendees.push_back(a);
+-            delegateesRef.push_back(&d->attendees.back());
+         }
+     }
+ 
++    //Build a list of attendee references
++    //These are pointers into d->attendees, so we MUST NOT modify that vector after this point!
++    std::vector<Kolab::Attendee*> delegateesRef;
++    foreach(const Attendee &a, delegatees) {
++        Attendee *attendee = getAttendee(a.contact());
++        Q_ASSERT(attendee);
++        delegateesRef.push_back(attendee);
++    }
++
+     std::vector<Kolab::Attendee*> delegatorsRef;
+     foreach(const Attendee& a, delegators) {
+         if (Attendee *attendee = getAttendee(a.contact())) {
+-- 
+2.31.1
+

0001-Fix-use-after-free-bug.patch Added

@@ -0,0 +1,31 @@
+From c801b50bf541a8825bc640438339731d5e08b34a Mon Sep 17 00:00:00 2001
+From: Christoph Erhardt <kolab@sicherha.de>
+Date: Sun, 14 Feb 2021 18:27:41 +0100
+Subject: [PATCH] Fix use-after-free bug
+
+QByteArray::fromRawData() does not copy the contents of the source
+buffer. If the resulting QByteArray object lives longer than the source
+buffer, we run into use-after-free problems.
+
+In this particular instance, the source data resides in a temporary
+rvalue object.
+---
+ conversion/kcalconversion.cpp | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/conversion/kcalconversion.cpp b/conversion/kcalconversion.cpp
+index b06a5d0..7a5077e 100644
+--- a/conversion/kcalconversion.cpp
++++ b/conversion/kcalconversion.cpp
+@@ -316,7 +316,7 @@ void setIncidence(KCalCore::Incidence &i, const T &e)
+         if (!a.uri().empty()) {
+             ptr = KCalCore::Attachment::Ptr(new KCalCore::Attachment(fromStdString(a.uri()), fromStdString(a.mimetype())));
+         } else {
+-            ptr = KCalCore::Attachment::Ptr(new KCalCore::Attachment(QByteArray::fromRawData(a.data().c_str(), a.data().size()), fromStdString(a.mimetype())));
++            ptr = KCalCore::Attachment::Ptr(new KCalCore::Attachment(QByteArray(a.data().c_str(), a.data().size()), fromStdString(a.mimetype())));
+         }
+         if (!a.label().empty()) {
+             ptr->setLabel(fromStdString(a.label()));
+-- 
+2.31.1
+

debian.changelog Changed

debian.series Changed

libkolab.dsc Changed

Build Results
RPM Lint

Refresh

Comments for request 2797 0

Request History

sicherha created request almost 4 years ago

Fix memory-safety bugs

sicherha accepted request over 3 years ago