From 98ac2b34f979cf10da24c984e690dabf7b34794b Mon Sep 17 00:00:00 2001 From: Christian Grothoff Date: Sun, 13 May 2018 21:17:12 +0200 Subject: [PATCH] add some extra GNS-record well-formedness checks if logging is enabled --- src/dns/dnsparser.c | 8 +++- src/gns/gns_api.c | 4 +- src/gnsrecord/gnsrecord.c | 12 ++---- src/gnsrecord/gnsrecord_crypto.c | 29 ++++++++----- src/gnsrecord/gnsrecord_serialization.c | 42 +++++++++++++++++++ src/namestore/gnunet-service-namestore.c | 22 +++++----- .../gnunet-service-zonemaster-monitor.c | 18 ++++---- src/zonemaster/gnunet-service-zonemaster.c | 30 +++++++------ 8 files changed, 104 insertions(+), 61 deletions(-) diff --git a/src/dns/dnsparser.c b/src/dns/dnsparser.c index 7e200ee7c..6f9a24b7e 100644 --- a/src/dns/dnsparser.c +++ b/src/dns/dnsparser.c @@ -815,7 +815,9 @@ GNUNET_DNSPARSER_builder_add_name (char *dst, return GNUNET_SYSERR; if (IDNA_SUCCESS != - (rc = idna_to_ascii_8z (name, &idna_start, IDNA_ALLOW_UNASSIGNED))) + (rc = idna_to_ascii_8z (name, + &idna_start, + IDNA_ALLOW_UNASSIGNED))) { GNUNET_log (GNUNET_ERROR_TYPE_WARNING, _("Failed to convert UTF-8 name `%s' to DNS IDNA format: %s\n"), @@ -841,7 +843,9 @@ GNUNET_DNSPARSER_builder_add_name (char *dst, goto fail; /* segment too long or empty */ } dst[pos++] = (char) (uint8_t) len; - GNUNET_memcpy (&dst[pos], idna_name, len); + GNUNET_memcpy (&dst[pos], + idna_name, + len); pos += len; idna_name += len + 1; /* also skip dot */ } diff --git a/src/gns/gns_api.c b/src/gns/gns_api.c index ff67f0205..ed30fa44c 100644 --- a/src/gns/gns_api.c +++ b/src/gns/gns_api.c @@ -157,7 +157,7 @@ check_result (void *cls, size_t mlen = ntohs (lookup_msg->header.size) - sizeof (*lookup_msg); uint32_t rd_count = ntohl (lookup_msg->rd_count); struct GNUNET_GNSRECORD_Data rd[rd_count]; - + (void) cls; if (GNUNET_SYSERR == GNUNET_GNSRECORD_records_deserialize (mlen, @@ -201,7 +201,7 @@ handle_result (void *cls, return; proc = lr->lookup_proc; proc_cls = lr->proc_cls; - + GNUNET_assert (GNUNET_OK == GNUNET_GNSRECORD_records_deserialize (mlen, (const char*) &lookup_msg[1], diff --git a/src/gnsrecord/gnsrecord.c b/src/gnsrecord/gnsrecord.c index 8fc039fc6..ece1665fc 100644 --- a/src/gnsrecord/gnsrecord.c +++ b/src/gnsrecord/gnsrecord.c @@ -114,10 +114,9 @@ init () void __attribute__ ((destructor)) GNSRECORD_fini () { - unsigned int i; struct Plugin *plugin; - for (i = 0; i < num_plugins; i++) + for (unsigned int i = 0; i < num_plugins; i++) { plugin = gns_plugins[i]; GNUNET_break (NULL == @@ -146,12 +145,11 @@ GNUNET_GNSRECORD_value_to_string (uint32_t type, const void *data, size_t data_size) { - unsigned int i; struct Plugin *plugin; char *ret; init (); - for (i = 0; i < num_plugins; i++) + for (unsigned int i = 0; i < num_plugins; i++) { plugin = gns_plugins[i]; if (NULL != (ret = plugin->api->value_to_string (plugin->api->cls, @@ -180,11 +178,10 @@ GNUNET_GNSRECORD_string_to_value (uint32_t type, void **data, size_t *data_size) { - unsigned int i; struct Plugin *plugin; init (); - for (i = 0; i < num_plugins; i++) + for (unsigned int i = 0; i < num_plugins; i++) { plugin = gns_plugins[i]; if (GNUNET_OK == plugin->api->string_to_value (plugin->api->cls, @@ -234,14 +231,13 @@ GNUNET_GNSRECORD_typename_to_number (const char *dns_typename) const char * GNUNET_GNSRECORD_number_to_typename (uint32_t type) { - unsigned int i; struct Plugin *plugin; const char * ret; if (GNUNET_GNSRECORD_TYPE_ANY == type) return "ANY"; init (); - for (i = 0; i < num_plugins; i++) + for (unsigned int i = 0; i < num_plugins; i++) { plugin = gns_plugins[i]; if (NULL != (ret = plugin->api->number_to_typename (plugin->api->cls, diff --git a/src/gnsrecord/gnsrecord_crypto.c b/src/gnsrecord/gnsrecord_crypto.c index cebc842f3..6d59a545a 100644 --- a/src/gnsrecord/gnsrecord_crypto.c +++ b/src/gnsrecord/gnsrecord_crypto.c @@ -1,6 +1,6 @@ /* This file is part of GNUnet. - Copyright (C) 2009-2013 GNUnet e.V. + Copyright (C) 2009-2013, 2018 GNUnet e.V. GNUnet is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published @@ -94,7 +94,7 @@ block_create (const struct GNUNET_CRYPTO_EcdsaPrivateKey *key, struct GNUNET_CRYPTO_EcdsaPrivateKey *dkey; struct GNUNET_CRYPTO_SymmetricInitializationVector iv; struct GNUNET_CRYPTO_SymmetricSessionKey skey; - struct GNUNET_GNSRECORD_Data rdc[rd_count]; + struct GNUNET_GNSRECORD_Data rdc[GNUNET_NZL(rd_count)]; uint32_t rd_count_nbo; struct GNUNET_TIME_Absolute now; @@ -246,6 +246,7 @@ GNUNET_GNSRECORD_block_create2 (const struct GNUNET_CRYPTO_EcdsaPrivateKey *key, GNUNET_CRYPTO_ecdsa_key_get_public (key, &line->pkey); } +#undef CSIZE return block_create (key, &line->pkey, expire, @@ -304,18 +305,21 @@ GNUNET_GNSRECORD_block_decrypt (const struct GNUNET_GNSRECORD_Block *block, GNUNET_break_op (0); return GNUNET_SYSERR; } - derive_block_aes_key (&iv, &skey, label, zone_key); + derive_block_aes_key (&iv, + &skey, + label, + zone_key); { char payload[payload_len]; uint32_t rd_count; GNUNET_break (payload_len == GNUNET_CRYPTO_symmetric_decrypt (&block[1], payload_len, - &skey, &iv, - payload)); + &skey, &iv, + payload)); GNUNET_memcpy (&rd_count, - payload, - sizeof (uint32_t)); + payload, + sizeof (uint32_t)); rd_count = ntohl (rd_count); if (rd_count > 2048) { @@ -324,7 +328,7 @@ GNUNET_GNSRECORD_block_decrypt (const struct GNUNET_GNSRECORD_Block *block, return GNUNET_SYSERR; } { - struct GNUNET_GNSRECORD_Data rd[rd_count]; + struct GNUNET_GNSRECORD_Data rd[GNUNET_NZL(rd_count)]; unsigned int j; struct GNUNET_TIME_Absolute now; @@ -359,10 +363,13 @@ GNUNET_GNSRECORD_block_decrypt (const struct GNUNET_GNSRECORD_Block *block, continue; if (rd[i].expiration_time < now.abs_value_us) include_record = GNUNET_NO; /* Shadow record is expired */ - if ((rd[k].record_type == rd[i].record_type) - && (rd[k].expiration_time >= now.abs_value_us) - && (0 == (rd[k].flags & GNUNET_GNSRECORD_RF_SHADOW_RECORD))) + if ( (rd[k].record_type == rd[i].record_type) && + (rd[k].expiration_time >= now.abs_value_us) && + (0 == (rd[k].flags & GNUNET_GNSRECORD_RF_SHADOW_RECORD)) ) + { include_record = GNUNET_NO; /* We have a non-expired, non-shadow record of the same type */ + break; + } } if (GNUNET_YES == include_record) { diff --git a/src/gnsrecord/gnsrecord_serialization.c b/src/gnsrecord/gnsrecord_serialization.c index 56521945d..1db27464f 100644 --- a/src/gnsrecord/gnsrecord_serialization.c +++ b/src/gnsrecord/gnsrecord_serialization.c @@ -127,17 +127,38 @@ GNUNET_GNSRECORD_records_serialize (unsigned int rd_count, rec.record_type = htonl (rd[i].record_type); rec.flags = htonl (rd[i].flags); if (off + sizeof (rec) > dest_size) + { + GNUNET_break (0); return -1; + } GNUNET_memcpy (&dest[off], &rec, sizeof (rec)); off += sizeof (rec); if (off + rd[i].data_size > dest_size) + { + GNUNET_break (0); return -1; + } GNUNET_memcpy (&dest[off], rd[i].data, rd[i].data_size); off += rd[i].data_size; +#if GNUNET_EXTRA_LOGGING + { + char *str; + + str = GNUNET_GNSRECORD_value_to_string (rd[i].record_type, + rd[i].data, + rd[i].data_size); + if (NULL == str) + { + GNUNET_break_op (0); + return GNUNET_SYSERR; + } + GNUNET_free (str); + } +#endif } return off; } @@ -165,7 +186,10 @@ GNUNET_GNSRECORD_records_deserialize (size_t len, for (unsigned int i=0;i len) + { + GNUNET_break_op (0); return GNUNET_SYSERR; + } GNUNET_memcpy (&rec, &src[off], sizeof (rec)); @@ -175,9 +199,27 @@ GNUNET_GNSRECORD_records_deserialize (size_t len, dest[i].flags = ntohl (rec.flags); off += sizeof (rec); if (off + dest[i].data_size > len) + { + GNUNET_break_op (0); return GNUNET_SYSERR; + } dest[i].data = &src[off]; off += dest[i].data_size; +#if GNUNET_EXTRA_LOGGING + { + char *str; + + str = GNUNET_GNSRECORD_value_to_string (dest[i].record_type, + dest[i].data, + dest[i].data_size); + if (NULL == str) + { + GNUNET_break_op (0); + return GNUNET_SYSERR; + } + GNUNET_free (str); + } +#endif LOG (GNUNET_ERROR_TYPE_DEBUG, "Deserialized record %u with flags %d and expiration time %llu\n", i, diff --git a/src/namestore/gnunet-service-namestore.c b/src/namestore/gnunet-service-namestore.c index 5e654dbde..8e88558de 100644 --- a/src/namestore/gnunet-service-namestore.c +++ b/src/namestore/gnunet-service-namestore.c @@ -550,18 +550,16 @@ merge_with_nick_records (const struct GNUNET_GNSRECORD_Data *nick_rd, uint64_t latest_expiration; size_t req; char *data; - int record_offset; size_t data_offset; (*rdc_res) = 1 + rd2_length; if (0 == 1 + rd2_length) { + GNUNET_break (0); (*rd_res) = NULL; return; } - req = 0; - for (unsigned int c=0; c< 1; c++) - req += sizeof (struct GNUNET_GNSRECORD_Data) + nick_rd[c].data_size; + 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); @@ -580,20 +578,19 @@ merge_with_nick_records (const struct GNUNET_GNSRECORD_Data *nick_rd, latest_expiration = rd2[c].expiration_time; (*rd_res)[c] = rd2[c]; (*rd_res)[c].data = (void *) &data[data_offset]; - GNUNET_memcpy ((void *) (*rd_res)[c].data, + GNUNET_memcpy (&data[data_offset], rd2[c].data, rd2[c].data_size); data_offset += (*rd_res)[c].data_size; } /* append nick */ - record_offset = rd2_length; - (*rd_res)[record_offset] = *nick_rd; - (*rd_res)[record_offset].expiration_time = latest_expiration; - (*rd_res)[record_offset].data = (void *) &data[data_offset]; - GNUNET_memcpy ((void *) (*rd_res)[record_offset].data, + (*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, nick_rd->data, nick_rd->data_size); - data_offset += (*rd_res)[record_offset].data_size; + data_offset += (*rd_res)[rd2_length].data_size; GNUNET_assert (req == (sizeof (struct GNUNET_GNSRECORD_Data)) * (*rdc_res) + data_offset); } @@ -647,7 +644,8 @@ send_lookup_response (struct NamestoreClient *nc, } name_len = strlen (name) + 1; - rd_ser_len = GNUNET_GNSRECORD_records_get_size (res_count, res); + rd_ser_len = GNUNET_GNSRECORD_records_get_size (res_count, + res); env = GNUNET_MQ_msg_extra (zir_msg, name_len + rd_ser_len, GNUNET_MESSAGE_TYPE_NAMESTORE_RECORD_RESULT); diff --git a/src/zonemaster/gnunet-service-zonemaster-monitor.c b/src/zonemaster/gnunet-service-zonemaster-monitor.c index 46feb117f..f7ae55ba7 100644 --- a/src/zonemaster/gnunet-service-zonemaster-monitor.c +++ b/src/zonemaster/gnunet-service-zonemaster-monitor.c @@ -214,16 +214,14 @@ convert_records_for_export (const struct GNUNET_GNSRECORD_Data *rd, rd_public_count = 0; now = GNUNET_TIME_absolute_get (); for (unsigned int i=0;i