check return values from GNSRECORD_record_serialize/size always

6 files changed, 206 insertions, 87 deletions

diff --git a/src/namestore/gnunet-service-namestore.c b/src/namestore/gnunet-service-namestore.c
index 8e88558de..1c73e2408 100644
--- a/src/namestore/gnunet-service-namestore.c
+++ b/src/namestore/gnunet-service-namestore.c
@@ -551,6 +551,7 @@ merge_with_nick_records (const struct GNUNET_GNSRECORD_Data *nick_rd,
   size_t req;
   char *data;
   size_t data_offset;
+  struct GNUNET_GNSRECORD_Data *target;
 
   (*rdc_res) = 1 + rd2_length;
   if (0 == 1 + rd2_length)
@@ -560,38 +561,52 @@ merge_with_nick_records (const struct GNUNET_GNSRECORD_Data *nick_rd,
     return;
   }
   req = sizeof (struct GNUNET_GNSRECORD_Data) + nick_rd->data_size;
-  for (unsigned int c=0; c< rd2_length; c++)
-    req += sizeof (struct GNUNET_GNSRECORD_Data) + rd2[c].data_size;
-  (*rd_res) = GNUNET_malloc (req);
-  data = (char *) &(*rd_res)[1 + rd2_length];
+  for (unsigned int i=0; i<rd2_length; i++)
+  {
+    const struct GNUNET_GNSRECORD_Data *orig = &rd2[i];
+
+    if (req + sizeof (struct GNUNET_GNSRECORD_Data) + orig->data_size < req)
+    {
+      GNUNET_break (0);
+      (*rd_res) = NULL;
+      return;
+    }
+    req += sizeof (struct GNUNET_GNSRECORD_Data) + orig->data_size;
+  }
+  target = GNUNET_malloc (req);
+  (*rd_res) = target;
+  data = (char *) &target[1 + rd2_length];
   data_offset = 0;
   latest_expiration = 0;
-  for (unsigned int c=0; c< rd2_length; c++)
+  for (unsigned int i=0;i<rd2_length;i++)
   {
-    if (0 != (rd2[c].flags & GNUNET_GNSRECORD_RF_RELATIVE_EXPIRATION))
+    const struct GNUNET_GNSRECORD_Data *orig = &rd2[i];
+
+    if (0 != (orig->flags & GNUNET_GNSRECORD_RF_RELATIVE_EXPIRATION))
     {
-      if ((GNUNET_TIME_absolute_get().abs_value_us + rd2[c].expiration_time) >
-        latest_expiration)
-          latest_expiration = rd2[c].expiration_time;
+      if ((GNUNET_TIME_absolute_get().abs_value_us + orig->expiration_time) >
+          latest_expiration)
+        latest_expiration = orig->expiration_time;
     }
-    else if (rd2[c].expiration_time > latest_expiration)
-      latest_expiration = rd2[c].expiration_time;
-    (*rd_res)[c] = rd2[c];
-    (*rd_res)[c].data = (void *) &data[data_offset];
+    else if (orig->expiration_time > latest_expiration)
+      latest_expiration = orig->expiration_time;
+    target[i] = *orig;
+    target[i].data = (void *) &data[data_offset];
     GNUNET_memcpy (&data[data_offset],
-                   rd2[c].data,
-                   rd2[c].data_size);
-    data_offset += (*rd_res)[c].data_size;
+                   orig->data,
+                   orig->data_size);
+    data_offset += orig->data_size;
   }
   /* append nick */
-  (*rd_res)[rd2_length] = *nick_rd;
-  (*rd_res)[rd2_length].expiration_time = latest_expiration;
-  (*rd_res)[rd2_length].data = (void *) &data[data_offset];
-  GNUNET_memcpy ((void *) (*rd_res)[rd2_length].data,
+  target[rd2_length] = *nick_rd;
+  target[rd2_length].expiration_time = latest_expiration;
+  target[rd2_length].data = (void *) &data[data_offset];
+  GNUNET_memcpy (&data[data_offset],
                  nick_rd->data,
                  nick_rd->data_size);
-  data_offset += (*rd_res)[rd2_length].data_size;
-  GNUNET_assert (req == (sizeof (struct GNUNET_GNSRECORD_Data)) * (*rdc_res) + data_offset);
+  data_offset += nick_rd->data_size;
+  GNUNET_assert (req ==
+                 (sizeof (struct GNUNET_GNSRECORD_Data)) * (*rdc_res) + data_offset);
 }
 
 
@@ -620,11 +635,16 @@ send_lookup_response (struct NamestoreClient *nc,
   struct GNUNET_GNSRECORD_Data *res;
   unsigned int res_count;
   size_t name_len;
-  size_t rd_ser_len;
+  ssize_t rd_ser_len;
   char *name_tmp;
   char *rd_ser;
 
   nick = get_nick_record (zone_key);
+
+  GNUNET_assert (-1 !=
+                 GNUNET_GNSRECORD_records_get_size (rd_count,
+                                                    rd));
+
   if ( (NULL != nick) &&
        (0 != strcmp (name,
                      GNUNET_GNS_EMPTY_LABEL_AT)))
@@ -643,26 +663,44 @@ send_lookup_response (struct NamestoreClient *nc,
     res = (struct GNUNET_GNSRECORD_Data *) rd;
   }
 
+  GNUNET_assert (-1 !=
+                 GNUNET_GNSRECORD_records_get_size (res_count,
+                                                    res));
+
+
   name_len = strlen (name) + 1;
   rd_ser_len = GNUNET_GNSRECORD_records_get_size (res_count,
                                                   res);
+  if (rd_ser_len < 0)
+  {
+    GNUNET_break (0);
+    GNUNET_SERVICE_client_drop (nc->client);
+    return;
+  }
+  if (rd_ser_len >= UINT16_MAX - name_len - sizeof (*zir_msg))
+  {
+    GNUNET_break (0);
+    GNUNET_SERVICE_client_drop (nc->client);
+    return;
+  }
   env = GNUNET_MQ_msg_extra (zir_msg,
                              name_len + rd_ser_len,
                              GNUNET_MESSAGE_TYPE_NAMESTORE_RECORD_RESULT);
   zir_msg->gns_header.r_id = htonl (request_id);
   zir_msg->name_len = htons (name_len);
   zir_msg->rd_count = htons (res_count);
-  zir_msg->rd_len = htons (rd_ser_len);
+  zir_msg->rd_len = htons ((uint16_t) rd_ser_len);
   zir_msg->private_key = *zone_key;
   name_tmp = (char *) &zir_msg[1];
   GNUNET_memcpy (name_tmp,
                  name,
                  name_len);
   rd_ser = &name_tmp[name_len];
-  GNUNET_GNSRECORD_records_serialize (res_count,
-                                      res,
-                                      rd_ser_len,
-                                      rd_ser);
+  GNUNET_assert (rd_ser_len ==
+                 GNUNET_GNSRECORD_records_serialize (res_count,
+                                                     res,
+                                                     rd_ser_len,
+                                                     rd_ser));
   GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
               "Sending RECORD_RESULT message with %u records\n",
               res_count);
@@ -1073,12 +1111,13 @@ struct RecordLookupContext
   /**
    * FIXME.
    */
-  size_t rd_ser_len;
+  ssize_t rd_ser_len;
 };
 
 
 /**
  * FIXME.
+ *
  * @param seq sequence number of the record
  */
 static void
@@ -1090,60 +1129,93 @@ lookup_it (void *cls,
            const struct GNUNET_GNSRECORD_Data *rd)
 {
   struct RecordLookupContext *rlc = cls;
-  struct GNUNET_GNSRECORD_Data *rd_res;
-  unsigned int rdc_res;
 
   (void) private_key;
   (void) seq;
-  if (0 == strcmp (label,
+  if (0 != strcmp (label,
                    rlc->label))
+    return;
+  rlc->found = GNUNET_YES;
+  if (0 == rd_count)
   {
-    rlc->found = GNUNET_YES;
-    if (0 != rd_count)
+    rlc->rd_ser_len = 0;
+    rlc->res_rd_count = 0;
+    rlc->res_rd = NULL;
+    return;
+  }
+  if ( (NULL != rlc->nick) &&
+       (0 != strcmp (label,
+                     GNUNET_GNS_EMPTY_LABEL_AT)) )
+  {
+    /* Merge */
+    struct GNUNET_GNSRECORD_Data *rd_res;
+    unsigned int rdc_res;
+
+    rd_res = NULL;
+    rdc_res = 0;
+    rlc->nick->flags = (rlc->nick->flags | GNUNET_GNSRECORD_RF_PRIVATE) ^ GNUNET_GNSRECORD_RF_PRIVATE;
+    merge_with_nick_records (rlc->nick,
+                             rd_count,
+                             rd,
+                             &rdc_res,
+                             &rd_res);
+    rlc->rd_ser_len = GNUNET_GNSRECORD_records_get_size (rdc_res,
+                                                         rd_res);
+    if (rlc->rd_ser_len < 0)
     {
-      if ( (NULL != rlc->nick) &&
-           (0 != strcmp (label,
-                         GNUNET_GNS_EMPTY_LABEL_AT)) )
-      {
-        /* Merge */
-        rd_res = NULL;
-        rdc_res = 0;
-        rlc->nick->flags = (rlc->nick->flags | GNUNET_GNSRECORD_RF_PRIVATE) ^ GNUNET_GNSRECORD_RF_PRIVATE;
-        merge_with_nick_records (rlc->nick,
-                                 rd_count,
-                                 rd,
-                                 &rdc_res,
-                                 &rd_res);
-        rlc->rd_ser_len = GNUNET_GNSRECORD_records_get_size (rdc_res,
-                                                             rd_res);
-        rlc->res_rd_count = rdc_res;
-        rlc->res_rd = GNUNET_malloc (rlc->rd_ser_len);
+      GNUNET_break (0);
+      GNUNET_free  (rd_res);
+      rlc->found = GNUNET_NO;
+      rlc->rd_ser_len = 0;
+      return;
+    }
+    rlc->res_rd_count = rdc_res;
+    rlc->res_rd = GNUNET_malloc (rlc->rd_ser_len);
+    if (rlc->rd_ser_len !=
         GNUNET_GNSRECORD_records_serialize (rdc_res,
                                             rd_res,
                                             rlc->rd_ser_len,
-                                            rlc->res_rd);
-
-        GNUNET_free  (rd_res);
-        GNUNET_free  (rlc->nick);
-        rlc->nick = NULL;
-      }
-      else
-      {
-        rlc->rd_ser_len = GNUNET_GNSRECORD_records_get_size (rd_count,
-                                                             rd);
-        rlc->res_rd_count = rd_count;
-        rlc->res_rd = GNUNET_malloc (rlc->rd_ser_len);
+                                            rlc->res_rd))
+    {
+      GNUNET_break (0);
+      GNUNET_free  (rlc->res_rd);
+      rlc->res_rd = NULL;
+      rlc->res_rd_count = 0;
+      rlc->rd_ser_len = 0;
+      GNUNET_free  (rd_res);
+      rlc->found = GNUNET_NO;
+      return;
+    }
+    GNUNET_free (rd_res);
+    GNUNET_free (rlc->nick);
+    rlc->nick = NULL;
+  }
+  else
+  {
+    rlc->rd_ser_len = GNUNET_GNSRECORD_records_get_size (rd_count,
+                                                         rd);
+    if (rlc->rd_ser_len < 0)
+    {
+      GNUNET_break (0);
+      rlc->found = GNUNET_NO;
+      rlc->rd_ser_len = 0;
+      return;
+    }
+    rlc->res_rd_count = rd_count;
+    rlc->res_rd = GNUNET_malloc (rlc->rd_ser_len);
+    if (rlc->rd_ser_len !=
         GNUNET_GNSRECORD_records_serialize (rd_count,
                                             rd,
                                             rlc->rd_ser_len,
-                                            rlc->res_rd);
-      }
-    }
-    else
+                                            rlc->res_rd))
     {
-      rlc->rd_ser_len = 0;
-      rlc->res_rd_count = 0;
+      GNUNET_break (0);
+      GNUNET_free  (rlc->res_rd);
       rlc->res_rd = NULL;
+      rlc->res_rd_count = 0;
+      rlc->rd_ser_len = 0;
+      rlc->found = GNUNET_NO;
+      return;
     }
   }
 }
@@ -1479,7 +1551,7 @@ handle_zone_to_name_it (void *cls,
   struct ZoneToNameResponseMessage *ztnr_msg;
   int16_t res;
   size_t name_len;
-  size_t rd_ser_len;
+  ssize_t rd_ser_len;
   size_t msg_size;
   char *name_tmp;
   char *rd_tmp;
@@ -1490,7 +1562,14 @@ handle_zone_to_name_it (void *cls,
               name);
   res = GNUNET_YES;
   name_len = (NULL == name) ? 0 : strlen (name) + 1;
-  rd_ser_len = GNUNET_GNSRECORD_records_get_size (rd_count, rd);
+  rd_ser_len = GNUNET_GNSRECORD_records_get_size (rd_count,
+                                                  rd);
+  if (rd_ser_len < 0)
+  {
+    GNUNET_break (0);
+    ztn_ctx->success = GNUNET_SYSERR;
+    return;
+  }
   msg_size = sizeof (struct ZoneToNameResponseMessage) + name_len + rd_ser_len;
   if (msg_size >= GNUNET_MAX_MESSAGE_SIZE)
   {
@@ -1513,10 +1592,11 @@ handle_zone_to_name_it (void *cls,
                  name,
                  name_len);
   rd_tmp = &name_tmp[name_len];
-  GNUNET_GNSRECORD_records_serialize (rd_count,
-                                      rd,
-                                      rd_ser_len,
-                                      rd_tmp);
+  GNUNET_assert (rd_ser_len ==
+                 GNUNET_GNSRECORD_records_serialize (rd_count,
+                                                     rd,
+                                                     rd_ser_len,
+                                                     rd_tmp));
   ztn_ctx->success = GNUNET_OK;
   GNUNET_MQ_send (ztn_ctx->nc->mq,
                   env);
diff --git a/src/namestore/namestore_api.c b/src/namestore/namestore_api.c
index 57bf8f81b..f2aaa43c8 100644
--- a/src/namestore/namestore_api.c
+++ b/src/namestore/namestore_api.c
@@ -1033,7 +1033,7 @@ GNUNET_NAMESTORE_records_store (struct GNUNET_NAMESTORE_Handle *h,
   struct GNUNET_MQ_Envelope *env;
   char *name_tmp;
   char *rd_ser;
-  size_t rd_ser_len;
+  ssize_t rd_ser_len;
   size_t name_len;
   uint32_t rid;
   struct RecordStoreMessage *msg;
@@ -1045,6 +1045,18 @@ GNUNET_NAMESTORE_records_store (struct GNUNET_NAMESTORE_Handle *h,
     GNUNET_break (0);
     return NULL;
   }
+  rd_ser_len = GNUNET_GNSRECORD_records_get_size (rd_count,
+                                                  rd);
+  if (rd_ser_len < 0)
+  {
+    GNUNET_break (0);
+    return NULL;
+  }
+  if (rd_ser_len > UINT16_MAX)
+  {
+    GNUNET_break (0);
+    return NULL;
+  }
   rid = get_op_id (h);
   qe = GNUNET_new (struct GNUNET_NAMESTORE_QueueEntry);
   qe->h = h;
@@ -1056,8 +1068,6 @@ GNUNET_NAMESTORE_records_store (struct GNUNET_NAMESTORE_Handle *h,
                                     qe);
 
   /* setup msg */
-  rd_ser_len = GNUNET_GNSRECORD_records_get_size (rd_count,
-                                                  rd);
   env = GNUNET_MQ_msg_extra (msg,
                              name_len + rd_ser_len,
                              GNUNET_MESSAGE_TYPE_NAMESTORE_RECORD_STORE);
@@ -1077,8 +1087,10 @@ GNUNET_NAMESTORE_records_store (struct GNUNET_NAMESTORE_Handle *h,
                                              rd,
                                              rd_ser_len,
                                              rd_ser);
-  if (0 > sret)
+  if ( (0 > sret) ||
+       (sret != rd_ser_len) )
   {
+    GNUNET_break (0);
     GNUNET_free (env);
     return NULL;
   }
diff --git a/src/namestore/namestore_api_monitor.c b/src/namestore/namestore_api_monitor.c
index 9ba90833b..16780ad0c 100644
--- a/src/namestore/namestore_api_monitor.c
+++ b/src/namestore/namestore_api_monitor.c
@@ -144,6 +144,11 @@ check_result (void *cls,
   rd_len = ntohs (lrm->rd_len);
   rd_count = ntohs (lrm->rd_count);
   name_len = ntohs (lrm->name_len);
+  if (name_len > MAX_NAME_LEN)
+  {
+    GNUNET_break (0);
+    return GNUNET_SYSERR;
+  }
   exp_lrm_len = sizeof (struct RecordResultMessage) + name_len + rd_len;
   if (lrm_len != exp_lrm_len)
   {
@@ -156,7 +161,7 @@ check_result (void *cls,
     return GNUNET_SYSERR;
   }
   name_tmp = (const char *) &lrm[1];
-  if ((name_tmp[name_len -1] != '\0') || (name_len > MAX_NAME_LEN))
+  if (name_tmp[name_len -1] != '\0')
   {
     GNUNET_break (0);
     return GNUNET_SYSERR;
diff --git a/src/namestore/plugin_namestore_flat.c b/src/namestore/plugin_namestore_flat.c
index 35d8424b4..95edb9d87 100644
--- a/src/namestore/plugin_namestore_flat.c
+++ b/src/namestore/plugin_namestore_flat.c
@@ -301,7 +301,7 @@ store_and_free_entries (void *cls,
   char *line;
   char *zone_private_key;
   char *record_data_b64;
-  size_t data_size;
+  ssize_t data_size;
 
   (void) key;
   GNUNET_STRINGS_base64_encode ((char*)entry->private_key,
@@ -309,6 +309,18 @@ store_and_free_entries (void *cls,
                                 &zone_private_key);
   data_size = GNUNET_GNSRECORD_records_get_size (entry->record_count,
                                                  entry->record_data);
+  if (data_size < 0)
+  {
+    GNUNET_break (0);
+    GNUNET_free (zone_private_key);
+    return GNUNET_SYSERR;
+  }
+  if (data_size >= UINT16_MAX)
+  {
+    GNUNET_break (0);
+    GNUNET_free (zone_private_key);
+    return GNUNET_SYSERR;
+  }
   {
     char data[data_size];
     ssize_t ret;
@@ -318,7 +330,7 @@ store_and_free_entries (void *cls,
                                               data_size,
                                               data);
     if ( (ret < 0) ||
-         (data_size != (size_t) ret) )
+         (data_size != ret) )
     {
       GNUNET_break (0);
       GNUNET_free (zone_private_key);
diff --git a/src/namestore/plugin_namestore_postgres.c b/src/namestore/plugin_namestore_postgres.c
index d7907b1a6..09fdd760f 100644
--- a/src/namestore/plugin_namestore_postgres.c
+++ b/src/namestore/plugin_namestore_postgres.c
@@ -220,7 +220,7 @@ namestore_postgres_store_records (void *cls,
   struct GNUNET_CRYPTO_EcdsaPublicKey pkey;
   uint64_t rvalue;
   uint32_t rd_count32 = (uint32_t) rd_count;
-  size_t data_size;
+  ssize_t data_size;
 
   memset (&pkey,
           0,
@@ -238,7 +238,12 @@ namestore_postgres_store_records (void *cls,
                                      UINT64_MAX);
   data_size = GNUNET_GNSRECORD_records_get_size (rd_count,
                                                  rd);
-  if (data_size > 64 * 65536)
+  if (data_size < 0)
+  {
+    GNUNET_break (0);
+    return GNUNET_SYSERR;
+  }
+  if (data_size >= UINT16_MAX)
   {
     GNUNET_break (0);
     return GNUNET_SYSERR;
@@ -287,7 +292,7 @@ namestore_postgres_store_records (void *cls,
                                               data_size,
                                               data);
     if ( (ret < 0) ||
-         (data_size != (size_t) ret) )
+         (data_size != ret) )
     {
       GNUNET_break (0);
       return GNUNET_SYSERR;
diff --git a/src/namestore/plugin_namestore_sqlite.c b/src/namestore/plugin_namestore_sqlite.c
index 34e548613..81391ce8a 100644
--- a/src/namestore/plugin_namestore_sqlite.c
+++ b/src/namestore/plugin_namestore_sqlite.c
@@ -313,7 +313,7 @@ namestore_sqlite_store_records (void *cls,
   int n;
   struct GNUNET_CRYPTO_EcdsaPublicKey pkey;
   uint64_t rvalue;
-  size_t data_size;
+  ssize_t data_size;
 
   memset (&pkey,
           0,
@@ -332,6 +332,11 @@ namestore_sqlite_store_records (void *cls,
                                      UINT64_MAX);
   data_size = GNUNET_GNSRECORD_records_get_size (rd_count,
                                                  rd);
+  if (data_size < 0)
+  {
+    GNUNET_break (0);
+    return GNUNET_SYSERR;
+  }
   if (data_size > 64 * 65536)
   {
     GNUNET_break (0);
@@ -352,7 +357,7 @@ namestore_sqlite_store_records (void *cls,
                                               data_size,
                                               data);
     if ( (ret < 0) ||
-         (data_size != (size_t) ret) )
+         (data_size != ret) )
     {
       GNUNET_break (0);
       return GNUNET_SYSERR;