gresolver: More robust parsing of DNS responses

 * Handle truncated responses, and invalid names

https://bugzilla.gnome.org/show_bug.cgi?id=675966

diff --git a/gio/gresolver.c b/gio/gresolver.c
index 811c971..efebd5a 100644 (file)
--- a/gio/gresolver.c
+++ b/gio/gresolver.c
@@ -994,18 +994,47 @@ _g_resolver_name_from_nameinfo (GInetAddress  *address,
 }
 
 #if defined(G_OS_UNIX)
+
+static gboolean
+parse_short (guchar  **p,
+             guchar   *end,
+             guint16  *value)
+{
+  if (*p + 2 > end)
+    return FALSE;
+  GETSHORT (*value, *p);
+  return TRUE;
+}
+
+static gboolean
+parse_long (guchar  **p,
+            guchar   *end,
+            guint32  *value)
+{
+  if (*p + 4 > end)
+    return FALSE;
+  GETLONG (*value, *p);
+  return TRUE;
+}
+
 static GVariant *
 parse_res_srv (guchar  *answer,
                guchar  *end,
-               guchar **p)
+               guchar  *p)
 {
   gchar namebuf[1024];
   guint16 priority, weight, port;
+  gint n;
+
+  if (!parse_short (&p, end, &priority) ||
+      !parse_short (&p, end, &weight) ||
+      !parse_short (&p, end, &port))
+    return NULL;
 
-  GETSHORT (priority, *p);
-  GETSHORT (weight, *p);
-  GETSHORT (port, *p);
-  *p += dn_expand (answer, end, *p, namebuf, sizeof (namebuf));
+  n = dn_expand (answer, end, p, namebuf, sizeof (namebuf));
+  if (n < 0)
+    return NULL;
+  *p += n;
 
   return g_variant_new ("(qqqs)",
                         priority,
@@ -1017,20 +1046,29 @@ parse_res_srv (guchar  *answer,
 static GVariant *
 parse_res_soa (guchar  *answer,
                guchar  *end,
-               guchar **p)
+               guchar  *p)
 {
   gchar mnamebuf[1024];
   gchar rnamebuf[1024];
   guint32 serial, refresh, retry, expire, ttl;
+  gint n;
 
-  *p += dn_expand (answer, end, *p, mnamebuf, sizeof (mnamebuf));
-  *p += dn_expand (answer, end, *p, rnamebuf, sizeof (rnamebuf));
+  n = dn_expand (answer, end, p, mnamebuf, sizeof (mnamebuf));
+  if (n < 0)
+    return NULL;
+  p += n;
 
-  GETLONG (serial, *p);
-  GETLONG (refresh, *p);
-  GETLONG (retry, *p);
-  GETLONG (expire, *p);
-  GETLONG (ttl, *p);
+  n = dn_expand (answer, end, p, rnamebuf, sizeof (rnamebuf));
+  if (n < 0)
+    return NULL;
+  p += n;
+
+  if (!parse_long (&p, end, &serial) ||
+      !parse_long (&p, end, &refresh) ||
+      !parse_long (&p, end, &retry) ||
+      !parse_long (&p, end, &expire) ||
+      !parse_long (&p, end, &ttl))
+    return NULL;
 
   return g_variant_new ("(ssuuuuu)",
                         mnamebuf,
@@ -1045,11 +1083,14 @@ parse_res_soa (guchar  *answer,
 static GVariant *
 parse_res_ns (guchar  *answer,
               guchar  *end,
-              guchar **p)
+              guchar  *p)
 {
   gchar namebuf[1024];
+  gint n;
 
-  *p += dn_expand (answer, end, *p, namebuf, sizeof (namebuf));
+  n = dn_expand (answer, end, p, namebuf, sizeof (namebuf));
+  if (n < 0)
+    return NULL;
 
   return g_variant_new ("(s)", namebuf);
 }
@@ -1057,14 +1098,19 @@ parse_res_ns (guchar  *answer,
 static GVariant *
 parse_res_mx (guchar  *answer,
               guchar  *end,
-              guchar **p)
+              guchar  *p)
 {
   gchar namebuf[1024];
   guint16 preference;
+  gint n;
 
-  GETSHORT (preference, *p);
+  if (!parse_short (&p, end, &preference))
+    return NULL;
 
-  *p += dn_expand (answer, end, *p, namebuf, sizeof (namebuf));
+  n = dn_expand (answer, end, p, namebuf, sizeof (namebuf));
+  if (n < 0)
+    return NULL;
+  p += n;
 
   return g_variant_new ("(qs)",
                         preference,
@@ -1074,24 +1120,22 @@ parse_res_mx (guchar  *answer,
 static GVariant *
 parse_res_txt (guchar  *answer,
                guchar  *end,
-               guchar **p)
+               guchar  *p)
 {
   GVariant *record;
   GPtrArray *array;
-  guchar *at = *p;
   gsize len;
 
   array = g_ptr_array_new_with_free_func (g_free);
-  while (at < end)
+  while (p < end)
     {
-      len = *(at++);
-      if (len > at - end)
+      len = *(p++);
+      if (len > p - end)
         break;
-      g_ptr_array_add (array, g_strndup ((gchar *)at, len));
-      at += len;
+      g_ptr_array_add (array, g_strndup ((gchar *)p, len));
+      p += len;
     }
 
-  *p = at;
   record = g_variant_new ("(@as)",
                           g_variant_new_strv ((const gchar **)array->pdata, array->len));
   g_ptr_array_free (array, TRUE);
@@ -1127,13 +1171,13 @@ _g_resolver_records_from_res_query (const gchar      *rrname,
                                     GError          **error)
 {
   gint count;
-  gchar namebuf[1024];
   guchar *end, *p;
   guint16 type, qclass, rdlength;
   guint32 ttl;
   HEADER *header;
   GList *records;
   GVariant *record;
+  gint n, i;
 
   if (len <= 0)
     {
@@ -1168,60 +1212,85 @@ _g_resolver_records_from_res_query (const gchar      *rrname,
 
   /* Skip query */
   count = ntohs (header->qdcount);
-  while (count-- && p < end)
+  for (i = 0; i < count && p < end; i++)
     {
-      p += dn_expand (answer, end, p, namebuf, sizeof (namebuf));
+      n = dn_skipname (p, end);
+      if (n < 0)
+        break;
+      p += n;
       p += 4;
+    }
 
-      /* To silence gcc warnings */
-      namebuf[0] = namebuf[1];
+  /* Incomplete response */
+  if (i < count)
+    {
+      g_set_error (error, G_RESOLVER_ERROR, G_RESOLVER_ERROR_TEMPORARY_FAILURE,
+                   _("Incomplete data received for '%s'"), rrname);
+      return NULL;
     }
 
   /* Read answers */
   count = ntohs (header->ancount);
-  while (count-- && p < end)
+  for (i = 0; i < count && p < end; i++)
     {
-      p += dn_expand (answer, end, p, namebuf, sizeof (namebuf));
-      GETSHORT (type, p);
-      GETSHORT (qclass, p);
-      GETLONG  (ttl, p);
+      n = dn_skipname (p, end);
+      if (n < 0)
+        break;
+      p += n;
+
+      if (!parse_short (&p, end, &type) ||
+          !parse_short (&p, end, &qclass) ||
+          !parse_long (&p, end, &ttl) ||
+          !parse_short (&p, end, &rdlength))
+        break;
+
       ttl = ttl; /* To avoid -Wunused-but-set-variable */
-      GETSHORT (rdlength, p);
 
-      if (type != rrtype || qclass != C_IN)
-        {
-          p += rdlength;
-          continue;
-        }
+      if (p + rdlength > end)
+        break;
 
-      switch (rrtype)
+      if (type == rrtype && qclass == C_IN)
         {
-        case T_SRV:
-          record = parse_res_srv (answer, end, &p);
-          break;
-        case T_MX:
-          record = parse_res_mx (answer, end, &p);
-          break;
-        case T_SOA:
-          record = parse_res_soa (answer, end, &p);
-          break;
-        case T_NS:
-          record = parse_res_ns (answer, end, &p);
-          break;
-        case T_TXT:
-          record = parse_res_txt (answer, p + rdlength, &p);
-          break;
-        default:
-          g_warn_if_reached ();
-          record = NULL;
-          break;
+          switch (rrtype)
+            {
+            case T_SRV:
+              record = parse_res_srv (answer, end, p);
+              break;
+            case T_MX:
+              record = parse_res_mx (answer, end, p);
+              break;
+            case T_SOA:
+              record = parse_res_soa (answer, end, p);
+              break;
+            case T_NS:
+              record = parse_res_ns (answer, end, p);
+              break;
+            case T_TXT:
+              record = parse_res_txt (answer, p + rdlength, p);
+              break;
+            default:
+              g_warn_if_reached ();
+              record = NULL;
+              break;
+            }
+
+          if (record != NULL)
+            records = g_list_prepend (records, record);
         }
 
-      if (record != NULL)
-        records = g_list_prepend (records, record);
+      p += rdlength;
     }
 
-    return records;
+  /* Somehow got a truncated response */
+  if (i < count)
+    {
+      g_list_free_full (records, (GDestroyNotify)g_variant_unref);
+      g_set_error (error, G_RESOLVER_ERROR, G_RESOLVER_ERROR_TEMPORARY_FAILURE,
+                   _("Incomplete data received for '%s'"), rrname);
+      return NULL;
+    }
+
+  return records;
 }
 
 #elif defined(G_OS_WIN32)