discovery: fix an use-after-free in SFB user credentials

The base64 decoded username and password strings given to
stun_usage_turn_create() should not freed immediately, since they remain
used when handling the following related inbound stun replies.

diff --git a/agent/candidate.h b/agent/candidate.h
index e556c16..3418219 100644 (file)
--- a/agent/candidate.h
+++ b/agent/candidate.h
@@ -141,6 +141,10 @@ typedef struct _TurnServer TurnServer;
  * @server: The #NiceAddress of the TURN server
  * @username: The TURN username
  * @password: The TURN password
+ * @decoded_username: The base64 decoded TURN username
+ * @decoded_password: The base64 decoded TURN password
+ * @decoded_username_len: The length of @decoded_username
+ * @decoded_password_len: The length of @decoded_password
  * @type: The #NiceRelayType of the server
  *
  * A structure to store the TURN relay settings
@@ -152,6 +156,10 @@ struct _TurnServer
   NiceAddress server;
   gchar *username;
   gchar *password;
+  uint8_t *decoded_username;
+  uint8_t *decoded_password;
+  gsize decoded_username_len;
+  gsize decoded_password_len;
   NiceRelayType type;
 };
 

diff --git a/agent/component.c b/agent/component.c
index 60ffe5e..536ee86 100644 (file)
--- a/agent/component.c
+++ b/agent/component.c
@@ -1482,6 +1482,10 @@ turn_server_new (const gchar *server_ip, guint server_port,
   }
   turn->username = g_strdup (username);
   turn->password = g_strdup (password);
+  turn->decoded_username =
+      g_base64_decode ((gchar *)username, &turn->decoded_username_len);
+  turn->decoded_password =
+      g_base64_decode ((gchar *)password, &turn->decoded_password_len);
   turn->type = type;
 
   return turn;
@@ -1503,6 +1507,8 @@ turn_server_unref (TurnServer *turn)
   if (turn->ref_count == 0) {
     g_free (turn->username);
     g_free (turn->password);
+    g_free (turn->decoded_username);
+    g_free (turn->decoded_password);
     g_slice_free (TurnServer, turn);
   }
 }

diff --git a/agent/conncheck.c b/agent/conncheck.c
index 62a3216..3c4c889 100644 (file)
--- a/agent/conncheck.c
+++ b/agent/conncheck.c
@@ -1551,8 +1551,10 @@ static void priv_turn_allocate_refresh_tick_unlocked (NiceAgent *agent,
 
   if (turn_compat == STUN_USAGE_TURN_COMPATIBILITY_MSN ||
       turn_compat == STUN_USAGE_TURN_COMPATIBILITY_OC2007) {
-    username = g_base64_decode ((gchar *)username, &username_len);
-    password = g_base64_decode ((gchar *)password, &password_len);
+    username = cand->candidate->turn->decoded_username;
+    password = cand->candidate->turn->decoded_password;
+    username_len = cand->candidate->turn->decoded_username_len;
+    password_len = cand->candidate->turn->decoded_password_len;
   }
 
   buffer_len = stun_usage_turn_create_refresh (&cand->stun_agent,
@@ -1562,12 +1564,6 @@ static void priv_turn_allocate_refresh_tick_unlocked (NiceAgent *agent,
       password, password_len,
       turn_compat);
 
-  if (turn_compat == STUN_USAGE_TURN_COMPATIBILITY_MSN ||
-      turn_compat == STUN_USAGE_TURN_COMPATIBILITY_OC2007) {
-    g_free (username);
-    g_free (password);
-  }
-
   nice_debug ("Agent %p : Sending allocate Refresh %zd", agent,
       buffer_len);
 

diff --git a/agent/discovery.c b/agent/discovery.c
index 7155dc3..014dc6c 100644 (file)
--- a/agent/discovery.c
+++ b/agent/discovery.c
@@ -248,8 +248,10 @@ static gboolean refresh_remove_async (NiceAgent *agent, CandidateRefresh *cand,
 
   if (turn_compat == STUN_USAGE_TURN_COMPATIBILITY_MSN ||
       turn_compat == STUN_USAGE_TURN_COMPATIBILITY_OC2007) {
-    username = g_base64_decode ((gchar *)username, &username_len);
-    password = g_base64_decode ((gchar *)password, &password_len);
+    username = cand->candidate->turn->decoded_username;
+    password = cand->candidate->turn->decoded_password;
+    username_len = cand->candidate->turn->decoded_username_len;
+    password_len = cand->candidate->turn->decoded_password_len;
   }
 
   buffer_len = stun_usage_turn_create_refresh (&cand->stun_agent,
@@ -271,12 +273,6 @@ static gboolean refresh_remove_async (NiceAgent *agent, CandidateRefresh *cand,
         (NiceTimeoutLockedCallback) on_refresh_remove_timeout, cand);
   }
 
-  if (turn_compat == STUN_USAGE_TURN_COMPATIBILITY_MSN ||
-      turn_compat == STUN_USAGE_TURN_COMPATIBILITY_OC2007) {
-    g_free (username);
-    g_free (password);
-  }
-
   cand->destroy_cb = cb;
   cand->destroy_cb_data = cb_data;
 
@@ -1155,8 +1151,10 @@ static gboolean priv_discovery_tick_unlocked (NiceAgent *agent)
 
           if (turn_compat == STUN_USAGE_TURN_COMPATIBILITY_MSN ||
               turn_compat == STUN_USAGE_TURN_COMPATIBILITY_OC2007) {
-            username = g_base64_decode ((gchar *)username, &username_len);
-            password = g_base64_decode ((gchar *)password, &password_len);
+            username = cand->turn->decoded_username;
+            password = cand->turn->decoded_password;
+            username_len = cand->turn->decoded_username_len;
+            password_len = cand->turn->decoded_password_len;
           }
 
           buffer_len = stun_usage_turn_create (&cand->stun_agent,
@@ -1167,12 +1165,6 @@ static gboolean priv_discovery_tick_unlocked (NiceAgent *agent)
               username, username_len,
               password, password_len,
               turn_compat);
-
-          if (turn_compat == STUN_USAGE_TURN_COMPATIBILITY_MSN ||
-              turn_compat == STUN_USAGE_TURN_COMPATIBILITY_OC2007) {
-            g_free (username);
-            g_free (password);
-          }
         }
 
        if (buffer_len > 0) {