diff --git a/src/basic/siphash24.h b/src/basic/siphash24.h index 90a6de00e4..0b3e845bf4 100644 --- a/src/basic/siphash24.h +++ b/src/basic/siphash24.h @@ -5,9 +5,9 @@ #include #include #include -#include #include +#include "string-util.h" #include "time-util.h" struct siphash { @@ -33,11 +33,15 @@ static inline void siphash24_compress_usec_t(usec_t in, struct siphash *state) { siphash24_compress(&in, sizeof in, state); } -static inline void siphash24_compress_string(const char *in, struct siphash *state) { - if (!in) +static inline void siphash24_compress_safe(const void *in, size_t inlen, struct siphash *state) { + if (inlen == 0) return; - siphash24_compress(in, strlen(in), state); + siphash24_compress(in, inlen, state); +} + +static inline void siphash24_compress_string(const char *in, struct siphash *state) { + siphash24_compress_safe(in, strlen_ptr(in), state); } uint64_t siphash24_finalize(struct siphash *state); diff --git a/src/fuzz/fuzz-nspawn-oci.options b/src/fuzz/fuzz-nspawn-oci.options new file mode 100644 index 0000000000..678d526b1e --- /dev/null +++ b/src/fuzz/fuzz-nspawn-oci.options @@ -0,0 +1,2 @@ +[libfuzzer] +max_len = 65536 diff --git a/src/fuzz/fuzz-nspawn-settings.options b/src/fuzz/fuzz-nspawn-settings.options new file mode 100644 index 0000000000..678d526b1e --- /dev/null +++ b/src/fuzz/fuzz-nspawn-settings.options @@ -0,0 +1,2 @@ +[libfuzzer] +max_len = 65536 diff --git a/src/fuzz/fuzz-udev-rules.options b/src/fuzz/fuzz-udev-rules.options new file mode 100644 index 0000000000..678d526b1e --- /dev/null +++ b/src/fuzz/fuzz-udev-rules.options @@ -0,0 +1,2 @@ +[libfuzzer] +max_len = 65536 diff --git a/src/fuzz/fuzz-unit-file.options b/src/fuzz/fuzz-unit-file.options new file mode 100644 index 0000000000..678d526b1e --- /dev/null +++ b/src/fuzz/fuzz-unit-file.options @@ -0,0 +1,2 @@ +[libfuzzer] +max_len = 65536 diff --git a/src/resolve/resolved-dns-answer.c b/src/resolve/resolved-dns-answer.c index 5b762a82e8..f5e50fcd84 100644 --- a/src/resolve/resolved-dns-answer.c +++ b/src/resolve/resolved-dns-answer.c @@ -8,18 +8,53 @@ #include "resolved-dns-dnssec.h" #include "string-util.h" +static void dns_answer_item_hash_func(const DnsAnswerItem *a, struct siphash *state) { + assert(a); + assert(state); + + siphash24_compress(&a->ifindex, sizeof(a->ifindex), state); + + dns_resource_record_hash_func(a->rr, state); +} + +static int dns_answer_item_compare_func(const DnsAnswerItem *a, const DnsAnswerItem *b) { + int r; + + assert(a); + assert(b); + + r = CMP(a->ifindex, b->ifindex); + if (r != 0) + return r; + + return dns_resource_record_compare_func(a->rr, b->rr); +} + +DEFINE_PRIVATE_HASH_OPS(dns_answer_item_hash_ops, DnsAnswerItem, dns_answer_item_hash_func, dns_answer_item_compare_func); + DnsAnswer *dns_answer_new(size_t n) { + _cleanup_set_free_ Set *s = NULL; DnsAnswer *a; if (n > UINT16_MAX) /* We can only place 64K RRs in an answer at max */ n = UINT16_MAX; + s = set_new(&dns_answer_item_hash_ops); + if (!s) + return NULL; + + /* Higher multipliers give slightly higher efficiency through hash collisions, but the gains + * quickly drop off after 2. */ + if (set_reserve(s, n * 2) < 0) + return NULL; + a = malloc0(offsetof(DnsAnswer, items) + sizeof(DnsAnswerItem) * n); if (!a) return NULL; a->n_ref = 1; a->n_allocated = n; + a->set_items = TAKE_PTR(s); return a; } @@ -30,6 +65,8 @@ static void dns_answer_flush(DnsAnswer *a) { if (!a) return; + a->set_items = set_free(a->set_items); + DNS_ANSWER_FOREACH(rr, a) dns_resource_record_unref(rr); @@ -46,6 +83,8 @@ static DnsAnswer *dns_answer_free(DnsAnswer *a) { DEFINE_TRIVIAL_REF_UNREF_FUNC(DnsAnswer, dns_answer, dns_answer_free); static int dns_answer_add_raw(DnsAnswer *a, DnsResourceRecord *rr, int ifindex, DnsAnswerFlags flags) { + int r; + assert(rr); if (!a) @@ -54,12 +93,21 @@ static int dns_answer_add_raw(DnsAnswer *a, DnsResourceRecord *rr, int ifindex, if (a->n_rrs >= a->n_allocated) return -ENOSPC; - a->items[a->n_rrs++] = (DnsAnswerItem) { - .rr = dns_resource_record_ref(rr), + a->items[a->n_rrs] = (DnsAnswerItem) { + .rr = rr, .ifindex = ifindex, .flags = flags, }; + r = set_put(a->set_items, &a->items[a->n_rrs]); + if (r < 0) + return r; + if (r == 0) + return -EEXIST; + + dns_resource_record_ref(rr); + a->n_rrs++; + return 1; } @@ -78,8 +126,7 @@ static int dns_answer_add_raw_all(DnsAnswer *a, DnsAnswer *source) { } int dns_answer_add(DnsAnswer *a, DnsResourceRecord *rr, int ifindex, DnsAnswerFlags flags) { - size_t i; - int r; + DnsAnswerItem tmp, *exist; assert(rr); @@ -88,36 +135,26 @@ int dns_answer_add(DnsAnswer *a, DnsResourceRecord *rr, int ifindex, DnsAnswerFl if (a->n_ref > 1) return -EBUSY; - for (i = 0; i < a->n_rrs; i++) { - if (a->items[i].ifindex != ifindex) - continue; - - r = dns_resource_key_equal(a->items[i].rr->key, rr->key); - if (r < 0) - return r; - if (r == 0) - continue; + tmp = (DnsAnswerItem) { + .rr = rr, + .ifindex = ifindex, + }; + exist = set_get(a->set_items, &tmp); + if (exist) { /* There's already an RR of the same RRset in place! Let's see if the TTLs more or less * match. We don't really care if they match precisely, but we do care whether one is 0 and * the other is not. See RFC 2181, Section 5.2. */ - if ((rr->ttl == 0) != (a->items[i].rr->ttl == 0)) + if ((rr->ttl == 0) != (exist->rr->ttl == 0)) return -EINVAL; - r = dns_resource_record_payload_equal(a->items[i].rr, rr); - if (r < 0) - return r; - if (r == 0) - continue; - - /* Entry already exists, keep the entry with the higher RR. */ - if (rr->ttl > a->items[i].rr->ttl) { - dns_resource_record_ref(rr); - dns_resource_record_unref(a->items[i].rr); - a->items[i].rr = rr; + /* Entry already exists, keep the entry with the higher TTL. */ + if (rr->ttl > exist->rr->ttl) { + dns_resource_record_unref(exist->rr); + exist->rr = dns_resource_record_ref(rr); } - a->items[i].flags |= flags; + exist->flags |= flags; return 0; } diff --git a/src/resolve/resolved-dns-answer.h b/src/resolve/resolved-dns-answer.h index fd94c516de..d73525cedd 100644 --- a/src/resolve/resolved-dns-answer.h +++ b/src/resolve/resolved-dns-answer.h @@ -6,6 +6,7 @@ typedef struct DnsAnswerItem DnsAnswerItem; #include "macro.h" #include "resolved-dns-rr.h" +#include "set.h" /* A simple array of resource records. We keep track of the * originating ifindex for each RR where that makes sense, so that we @@ -30,6 +31,7 @@ struct DnsAnswerItem { struct DnsAnswer { unsigned n_ref; + Set *set_items; /* Used by dns_answer_add() for optimization. */ size_t n_rrs, n_allocated; DnsAnswerItem items[0]; }; diff --git a/src/resolve/resolved-dns-packet.c b/src/resolve/resolved-dns-packet.c index b4eb5efae7..ede499f486 100644 --- a/src/resolve/resolved-dns-packet.c +++ b/src/resolve/resolved-dns-packet.c @@ -2153,7 +2153,7 @@ static int dns_packet_extract_question(DnsPacket *p, DnsQuestion **ret_question) return log_oom(); r = set_reserve(keys, n * 2); /* Higher multipliers give slightly higher efficiency through - * hash collisions, but the gains quickly drop of after 2. */ + * hash collisions, but the gains quickly drop off after 2. */ if (r < 0) return r; diff --git a/src/resolve/resolved-dns-rr.c b/src/resolve/resolved-dns-rr.c index 219f66451a..19e075479f 100644 --- a/src/resolve/resolved-dns-rr.c +++ b/src/resolve/resolved-dns-rr.c @@ -294,21 +294,17 @@ static void dns_resource_key_hash_func(const DnsResourceKey *k, struct siphash * } static int dns_resource_key_compare_func(const DnsResourceKey *x, const DnsResourceKey *y) { - int ret; + int r; - ret = dns_name_compare_func(dns_resource_key_name(x), dns_resource_key_name(y)); - if (ret != 0) - return ret; + r = dns_name_compare_func(dns_resource_key_name(x), dns_resource_key_name(y)); + if (r != 0) + return r; - ret = CMP(x->type, y->type); - if (ret != 0) - return ret; + r = CMP(x->type, y->type); + if (r != 0) + return r; - ret = CMP(x->class, y->class); - if (ret != 0) - return ret; - - return 0; + return CMP(x->class, y->class); } DEFINE_HASH_OPS(dns_resource_key_hash_ops, DnsResourceKey, dns_resource_key_hash_func, dns_resource_key_compare_func); @@ -1373,7 +1369,7 @@ void dns_resource_record_hash_func(const DnsResourceRecord *rr, struct siphash * DnsTxtItem *j; LIST_FOREACH(items, j, rr->txt.items) { - siphash24_compress(j->data, j->length, state); + siphash24_compress_safe(j->data, j->length, state); /* Add an extra NUL byte, so that "a" followed by "b" doesn't result in the same hash as "ab" * followed by "". */ @@ -1418,14 +1414,14 @@ void dns_resource_record_hash_func(const DnsResourceRecord *rr, struct siphash * case DNS_TYPE_SSHFP: siphash24_compress(&rr->sshfp.algorithm, sizeof(rr->sshfp.algorithm), state); siphash24_compress(&rr->sshfp.fptype, sizeof(rr->sshfp.fptype), state); - siphash24_compress(rr->sshfp.fingerprint, rr->sshfp.fingerprint_size, state); + siphash24_compress_safe(rr->sshfp.fingerprint, rr->sshfp.fingerprint_size, state); break; case DNS_TYPE_DNSKEY: siphash24_compress(&rr->dnskey.flags, sizeof(rr->dnskey.flags), state); siphash24_compress(&rr->dnskey.protocol, sizeof(rr->dnskey.protocol), state); siphash24_compress(&rr->dnskey.algorithm, sizeof(rr->dnskey.algorithm), state); - siphash24_compress(rr->dnskey.key, rr->dnskey.key_size, state); + siphash24_compress_safe(rr->dnskey.key, rr->dnskey.key_size, state); break; case DNS_TYPE_RRSIG: @@ -1437,7 +1433,7 @@ void dns_resource_record_hash_func(const DnsResourceRecord *rr, struct siphash * siphash24_compress(&rr->rrsig.inception, sizeof(rr->rrsig.inception), state); siphash24_compress(&rr->rrsig.key_tag, sizeof(rr->rrsig.key_tag), state); dns_name_hash_func(rr->rrsig.signer, state); - siphash24_compress(rr->rrsig.signature, rr->rrsig.signature_size, state); + siphash24_compress_safe(rr->rrsig.signature, rr->rrsig.signature_size, state); break; case DNS_TYPE_NSEC: @@ -1451,15 +1447,15 @@ void dns_resource_record_hash_func(const DnsResourceRecord *rr, struct siphash * siphash24_compress(&rr->ds.key_tag, sizeof(rr->ds.key_tag), state); siphash24_compress(&rr->ds.algorithm, sizeof(rr->ds.algorithm), state); siphash24_compress(&rr->ds.digest_type, sizeof(rr->ds.digest_type), state); - siphash24_compress(rr->ds.digest, rr->ds.digest_size, state); + siphash24_compress_safe(rr->ds.digest, rr->ds.digest_size, state); break; case DNS_TYPE_NSEC3: siphash24_compress(&rr->nsec3.algorithm, sizeof(rr->nsec3.algorithm), state); siphash24_compress(&rr->nsec3.flags, sizeof(rr->nsec3.flags), state); siphash24_compress(&rr->nsec3.iterations, sizeof(rr->nsec3.iterations), state); - siphash24_compress(rr->nsec3.salt, rr->nsec3.salt_size, state); - siphash24_compress(rr->nsec3.next_hashed_name, rr->nsec3.next_hashed_name_size, state); + siphash24_compress_safe(rr->nsec3.salt, rr->nsec3.salt_size, state); + siphash24_compress_safe(rr->nsec3.next_hashed_name, rr->nsec3.next_hashed_name_size, state); /* FIXME: We leave the bitmaps out */ break; @@ -1467,30 +1463,30 @@ void dns_resource_record_hash_func(const DnsResourceRecord *rr, struct siphash * siphash24_compress(&rr->tlsa.cert_usage, sizeof(rr->tlsa.cert_usage), state); siphash24_compress(&rr->tlsa.selector, sizeof(rr->tlsa.selector), state); siphash24_compress(&rr->tlsa.matching_type, sizeof(rr->tlsa.matching_type), state); - siphash24_compress(rr->tlsa.data, rr->tlsa.data_size, state); + siphash24_compress_safe(rr->tlsa.data, rr->tlsa.data_size, state); break; case DNS_TYPE_CAA: siphash24_compress(&rr->caa.flags, sizeof(rr->caa.flags), state); string_hash_func(rr->caa.tag, state); - siphash24_compress(rr->caa.value, rr->caa.value_size, state); + siphash24_compress_safe(rr->caa.value, rr->caa.value_size, state); break; case DNS_TYPE_OPENPGPKEY: default: - siphash24_compress(rr->generic.data, rr->generic.data_size, state); + siphash24_compress_safe(rr->generic.data, rr->generic.data_size, state); break; } } -static int dns_resource_record_compare_func(const DnsResourceRecord *x, const DnsResourceRecord *y) { +int dns_resource_record_compare_func(const DnsResourceRecord *x, const DnsResourceRecord *y) { int r; r = dns_resource_key_compare_func(x->key, y->key); if (r != 0) return r; - if (dns_resource_record_equal(x, y)) + if (dns_resource_record_payload_equal(x, y) > 0) return 0; /* We still use CMP() here, even though don't implement proper diff --git a/src/resolve/resolved-dns-rr.h b/src/resolve/resolved-dns-rr.h index 59b3a70179..aa17d6d391 100644 --- a/src/resolve/resolved-dns-rr.h +++ b/src/resolve/resolved-dns-rr.h @@ -330,6 +330,7 @@ DnsTxtItem *dns_txt_item_copy(DnsTxtItem *i); int dns_txt_item_new_empty(DnsTxtItem **ret); void dns_resource_record_hash_func(const DnsResourceRecord *i, struct siphash *state); +int dns_resource_record_compare_func(const DnsResourceRecord *x, const DnsResourceRecord *y); extern const struct hash_ops dns_resource_key_hash_ops; extern const struct hash_ops dns_resource_record_hash_ops; diff --git a/test/fuzz/fuzz-dns-packet/oss-fuzz-19227 b/test/fuzz/fuzz-dns-packet/oss-fuzz-19227 new file mode 100644 index 0000000000..62828e7446 Binary files /dev/null and b/test/fuzz/fuzz-dns-packet/oss-fuzz-19227 differ