From e5f3df8ab02574d7edcce9ceef4622569edb6f92 Mon Sep 17 00:00:00 2001
From: Milan Crha <mcrha@redhat.com>
Date: Thu, 3 Jan 2019 12:53:04 +0100
Subject: [PATCH] Improve thread safety of icaltimezone_load_builtin_timezone()

Even the function does test whether the passed-in zone has set
the component, it doesn't retest it when it acquires the lock, but
other thread could already assign the component, which can cause
use-after-free in certain thread interleaving.
---
 src/libical/icaltimezone.c   |  6 ++++
 src/test/builtin_timezones.c | 54 ++++++++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+)

diff --git a/src/libical/icaltimezone.c b/src/libical/icaltimezone.c
index 12f6c291..3c4bf14d 100644
--- a/src/libical/icaltimezone.c
+++ b/src/libical/icaltimezone.c
@@ -1806,6 +1806,12 @@ static void icaltimezone_load_builtin_timezone(icaltimezone *zone)
 
     icaltimezone_builtin_lock();
 
+    /* Try again, maybe it had been set by other thread while waiting for the lock */
+    if (zone->component) {
+        icaltimezone_builtin_unlock();
+        return;
+    }
+
     /* If the location isn't set, it isn't a builtin timezone. */
     if (!zone->location || !zone->location[0]) {
         icaltimezone_builtin_unlock();
diff --git a/src/test/builtin_timezones.c b/src/test/builtin_timezones.c
index c2d36845..b0e472cb 100644
--- a/src/test/builtin_timezones.c
+++ b/src/test/builtin_timezones.c
@@ -20,10 +20,60 @@
 #include <config.h>
 #endif
 
+#ifdef HAVE_PTHREAD_H
+#include <pthread.h>
+#include <assert.h>
+#endif
+
 #include "libical/ical.h"
 
 #include <stdio.h>
 
+#if defined(HAVE_PTHREAD_H) && defined(HAVE_PTHREAD) && defined(HAVE_PTHREAD_CREATE)
+
+#define N_THREADS 20
+
+static const void *thread_comp = NULL;
+
+static void *
+thread_func(void *user_data)
+{
+    icaltimezone *zone = user_data;
+    icalcomponent *icalcomp;
+
+    if(!zone)
+        return NULL;
+
+    icalcomp = icaltimezone_get_component(zone);
+    if(!thread_comp)
+        thread_comp = icalcomp;
+    else
+        assert(thread_comp == icalcomp);
+    icalcomp = icalcomponent_new_clone(icalcomp);
+    icalcomponent_free(icalcomp);
+
+    return NULL;
+}
+
+static void
+test_get_component_threadsafety(void)
+{
+    pthread_t thread[N_THREADS];
+    icaltimezone *zone;
+    int ii;
+
+    zone = icaltimezone_get_builtin_timezone("Europe/London");
+
+    for(ii = 0; ii < N_THREADS; ii++) {
+        pthread_create(&thread[ii], NULL, thread_func, zone);
+    }
+
+    for(ii = 0; ii < N_THREADS; ii++) {
+        pthread_join(thread[ii], NULL);
+    }
+}
+#endif
+
 int main()
 {
     icalarray *builtin_timezones;
@@ -34,6 +84,10 @@ int main()
     set_zone_directory("../../zoneinfo");
     icaltimezone_set_tzid_prefix("/softwarestudio.org/");
 
+    #if defined(HAVE_PTHREAD_H) && defined(HAVE_PTHREAD) && defined(HAVE_PTHREAD_CREATE)
+    test_get_component_threadsafety();
+    #endif
+
     tt = icaltime_current_time_with_zone(icaltimezone_get_builtin_timezone("America/New_York"));
 
     tt.year = 2038;
-- 
2.17.0