From e1f22813b220384b2452d12860f6fea46208bc97 Mon Sep 17 00:00:00 2001
From: Bastien Nocera <hadess@hadess.net>
Date: Tue, 15 Oct 2019 11:10:13 +0200
Subject: [PATCH] weather: Fix possible use after free

Don't use potentially dangling data pointer after a cancelling a soup
function call. The user_data might already be freed at this point, and
we're getting called after gweather_weather_dispose() has been
triggered.

Closes: #34
---
 libgweather/weather-iwin.c  | 17 ++++++++++-------
 libgweather/weather-metar.c | 28 ++++++++++++++++------------
 libgweather/weather-owm.c   | 15 ++++++++++-----
 libgweather/weather-wx.c    | 19 ++++++++++++-------
 libgweather/weather-yrno.c  | 15 ++++++++++-----
 5 files changed, 58 insertions(+), 36 deletions(-)

diff --git a/libgweather/weather-iwin.c b/libgweather/weather-iwin.c
index c026391b..c3d414de 100644
--- a/libgweather/weather-iwin.c
+++ b/libgweather/weather-iwin.c
@@ -314,21 +314,24 @@ parseForecastXml (const char *buff, GWeatherInfo *master_info)
 static void
 iwin_finish (SoupSession *session, SoupMessage *msg, gpointer data)
 {
-    GWeatherInfo *info = (GWeatherInfo *)data;
+    GWeatherInfo *info;
     GWeatherInfoPrivate *priv;
     WeatherLocation *loc;
 
-    g_return_if_fail (info != NULL);
-
     if (!SOUP_STATUS_IS_SUCCESSFUL (msg->status_code)) {
         /* forecast data is not really interesting anyway ;) */
-	if (msg->status_code != SOUP_STATUS_CANCELLED)
-	    g_warning ("Failed to get IWIN forecast data: %d %s\n",
-		       msg->status_code, msg->reason_phrase);
-        _gweather_info_request_done (info, msg);
+	if (msg->status_code == SOUP_STATUS_CANCELLED) {
+	    g_debug ("Failed to get IWIN forecast data: %d %s\n",
+		     msg->status_code, msg->reason_phrase);
+	    return;
+	}
+	g_warning ("Failed to get IWIN forecast data: %d %s\n",
+		   msg->status_code, msg->reason_phrase);
+        _gweather_info_request_done (data, msg);
         return;
     }
 
+    info = data;
     priv = info->priv;
     loc = &priv->location;
 
diff --git a/libgweather/weather-metar.c b/libgweather/weather-metar.c
index 5f041cb2..fad2f2f9 100644
--- a/libgweather/weather-metar.c
+++ b/libgweather/weather-metar.c
@@ -577,31 +577,35 @@ metar_parse (gchar *metar, GWeatherInfo *info)
 static void
 metar_finish (SoupSession *session, SoupMessage *msg, gpointer data)
 {
-    GWeatherInfo *info = (GWeatherInfo *)data;
+    GWeatherInfo *info;
     GWeatherInfoPrivate *priv;
     WeatherLocation *loc;
     const gchar *p, *eoln;
     gchar *searchkey, *metar;
     gboolean success = FALSE;
 
-    g_return_if_fail (info != NULL);
-
-    priv = info->priv;
-   
     if (!SOUP_STATUS_IS_SUCCESSFUL (msg->status_code)) {
-	if (SOUP_STATUS_IS_TRANSPORT_ERROR (msg->status_code))
-	    priv->network_error = TRUE;
-	else {
-	    if (msg->status_code != SOUP_STATUS_CANCELLED)
-		/* Translators: %d is an error code, and %s the error string */
-		g_warning (_("Failed to get METAR data: %d %s.\n"),
-			   msg->status_code, msg->reason_phrase);
+        if (msg->status_code == SOUP_STATUS_CANCELLED) {
+	    g_debug ("Failed to get METAR data: %d %s.\n",
+		     msg->status_code, msg->reason_phrase);
+	    return;
+	}
+
+	info = data;
+	if (SOUP_STATUS_IS_TRANSPORT_ERROR (msg->status_code)) {
+	    info->priv->network_error = TRUE;
+	} else {
+	    /* Translators: %d is an error code, and %s the error string */
+	    g_warning (_("Failed to get METAR data: %d %s.\n"),
+		       msg->status_code, msg->reason_phrase);
 	}
 
 	_gweather_info_request_done (info, msg);
 	return;
     }
 
+    info = data;
+    priv = info->priv;
     loc = &priv->location;
 
     g_debug ("METAR data for %s", loc->code);
diff --git a/libgweather/weather-owm.c b/libgweather/weather-owm.c
index 2e9d8b4e..a58950c9 100644
--- a/libgweather/weather-owm.c
+++ b/libgweather/weather-owm.c
@@ -394,19 +394,24 @@ owm_finish (SoupSession *session,
             SoupMessage *msg,
             gpointer     user_data)
 {
-    GWeatherInfo *info = GWEATHER_INFO (user_data);
+    GWeatherInfo *info;
     GWeatherInfoPrivate *priv;
     WeatherLocation *loc;
 
     if (!SOUP_STATUS_IS_SUCCESSFUL (msg->status_code)) {
 	/* forecast data is not really interesting anyway ;) */
-	if (msg->status_code != SOUP_STATUS_CANCELLED)
-	    g_message ("Failed to get OpenWeatherMap forecast data: %d %s\n",
-		       msg->status_code, msg->reason_phrase);
-	_gweather_info_request_done (info, msg);
+	if (msg->status_code == SOUP_STATUS_CANCELLED) {
+	    g_debug ("Failed to get OpenWeatherMap forecast data: %d %s\n",
+		     msg->status_code, msg->reason_phrase);
+	    return;
+	}
+	g_warning ("Failed to get OpenWeatherMap forecast data: %d %s\n",
+		   msg->status_code, msg->reason_phrase);
+	_gweather_info_request_done (user_data, msg);
 	return;
     }
 
+    info = user_data;
     priv = info->priv;
     loc = &priv->location;
     g_debug ("owm data for %lf, %lf", loc->latitude, loc->longitude);
diff --git a/libgweather/weather-wx.c b/libgweather/weather-wx.c
index f1e57d7a..e2f9e0a6 100644
--- a/libgweather/weather-wx.c
+++ b/libgweather/weather-wx.c
@@ -25,22 +25,27 @@
 static void
 wx_finish (SoupSession *session, SoupMessage *msg, gpointer data)
 {
-    GWeatherInfo *info = (GWeatherInfo *)data;
+    GWeatherInfo *info;
     GWeatherInfoPrivate *priv;
     GdkPixbufAnimation *animation;
 
-    g_return_if_fail (info != NULL);
-    priv = info->priv;
-
     if (!SOUP_STATUS_IS_SUCCESSFUL (msg->status_code)) {
-	if (msg->status_code != SOUP_STATUS_CANCELLED)
-	    g_warning ("Failed to get radar map image: %d %s.\n",
+	if (msg->status_code == SOUP_STATUS_CANCELLED) {
+	    g_debug ("Failed to get radar map image: %d %s.\n",
 		       msg->status_code, msg->reason_phrase);
-	g_object_unref (priv->radar_loader);
+	    return;
+	}
+	g_warning ("Failed to get radar map image: %d %s.\n",
+		   msg->status_code, msg->reason_phrase);
+	info = data;
+	g_object_unref (info->priv->radar_loader);
 	_gweather_info_request_done (info, msg);
 	return;
     }
 
+    info = data;
+    priv = info->priv;
+
     gdk_pixbuf_loader_close (priv->radar_loader, NULL);
     animation = gdk_pixbuf_loader_get_animation (priv->radar_loader);
     if (animation != NULL) {
diff --git a/libgweather/weather-yrno.c b/libgweather/weather-yrno.c
index e47dd358..6293a13d 100644
--- a/libgweather/weather-yrno.c
+++ b/libgweather/weather-yrno.c
@@ -388,20 +388,25 @@ yrno_finish_new (SoupSession *session,
 		 SoupMessage *msg,
 		 gpointer     user_data)
 {
-    GWeatherInfo *info = GWEATHER_INFO (user_data);
+    GWeatherInfo *info;
     GWeatherInfoPrivate *priv;
     WeatherLocation *loc;
     guint num_forecasts;
 
     if (!SOUP_STATUS_IS_SUCCESSFUL (msg->status_code)) {
 	/* forecast data is not really interesting anyway ;) */
-	if (msg->status_code != SOUP_STATUS_CANCELLED)
-	    g_message ("Failed to get Yr.no forecast data: %d %s\n",
-		       msg->status_code, msg->reason_phrase);
-	_gweather_info_request_done (info, msg);
+	if (msg->status_code == SOUP_STATUS_CANCELLED) {
+	    g_debug ("Failed to get Yr.no forecast data: %d %s\n",
+		     msg->status_code, msg->reason_phrase);
+	    return;
+	}
+	g_message ("Failed to get Yr.no forecast data: %d %s\n",
+		   msg->status_code, msg->reason_phrase);
+	_gweather_info_request_done (user_data, msg);
 	return;
     }
 
+    info = user_data;
     priv = info->priv;
     loc = &priv->location;
     g_debug ("yrno data for %lf, %lf", loc->latitude, loc->longitude);
-- 
2.31.1

