rr: limit the amount of allowed rr descriptors

[xypiie]

A malicious DNS server could send an answer causing ldns to assume millions of rr descriptors.
This could cause ldns consuming hundrets of MiB's of memory which then could cause the OOM handler to step in and kill the process.

Thus we want to limit the amount of allowed descriptors to prevent this from being used as remote DoS attacks.

Issue found by Maciej Musial, Robert Rozanski and Monika Walendzik.

[xypiie]

I uploaded patches to reproduce the issue here:
https://github.com/xypiie/ldns/tree/huge_mem_reproducer

[wtoorop]

Thanks.

ldns_rr_desciptor_maximum returns the maximum number of rdata fields (see: https://nlnetlabs.nl/documentation/ldns/rr_8h.html#a903dbeb5f9d525abf9d9503102dc3cc8 ). Realistically, this can be maximally 65535 as limited by rdlength. (so not millions)

Your example in raw_answer_producing_huge_memload has an TXT RR with 16384 text elements (each containing no text). This resulted in allocating 16384 ldns_rdf structs of type LDNS_RDF_TYPE_STR, each allocated ldns_rdf is done by its own malloc. In addition, the array of _rdata_fields is reallocated to make room for the extra ldns_rdf each time one more is encountered.

On my system, your debugging program reports +- 209MiB memory usage for the raw_answer_producing_huge_memload packet. This is indeed rather large. If memory was optimally utilized, this should in theory (with how ldns handles data) on a 64 bit system just take 32 * 16384 = 0.5MiB (so 400 times less).

I think some optimizations could be made (for example trying to limit the number of needed reallocations, or using a region allocator like unbound and/or nsd do), but this is all quite involved. The example packet does contain a valid DNS RR, and I think ldns should be able to handle it.

If low memory usage is vital to you, you may have a look at sldns, which is a version of ldns (part of unbound) that doesn't take apart wireformat packets into malloced pieces in host format, but reads and writes wire and presentation format directly: https://github.com/NLnetLabs/unbound/tree/master/sldns

I'm willing to limit the number of rdata fields enabled and set with an option to configure, but packets crafted in other ways, for example to maximize the number of RRs in a RRset would have similar results (though less bad, since they have more content for each malloc).

[xypiie]

I meant that it's millions, because of the return UINT_MAX; which is triggered by the raw_answer_producing_huge_memload.

The following printf makes it visible on top of https://github.com/xypiie/ldns/tree/huge_mem_reproducer:

diff --git a/rr.c b/rr.c
index a37e8d01..3542ce8f 100644
--- a/rr.c
+++ b/rr.c
@@ -2768,8 +2768,10 @@ ldns_rr_descriptor_maximum(const ldns_rr_descriptor *descriptor)
        if (descriptor) {
                if (descriptor->_variable != LDNS_RDF_TYPE_NONE) {
                        /* Should really be SIZE_MAX... bad FreeBSD.  */
+                       printf("rr_descriptor_maximum from define UINT_MAX: %u 0x%x\n", UINT_MAX, UINT_MAX);
                        return UINT_MAX;
                } else {
+                       printf("rr_descriptor_maximum from descriptor: %u 0x%x\n", descriptor->_maximum, descriptor->_maximum);
                        return descriptor->_maximum;
                }
        } else {

output:

rr_descriptor_maximum from define UINT_MAX: 4294967295 0xffffffff
real: 220MiB, virt: 298MiB ldns_wire2rdf 188

Low memory usage is not the ultimate goal I'm looking for. Security is. Thus the point I wanted to make is, that a crafted response from a remote dns server will cause the memory usage to grow massively beyond the "normal" ~8MiB of e.g. drill. And this will be the case for all users of ldns library, when they get such an crafted response.

Yes, some optimization on the memory handling would be great.

What would be your proposal to "limit the number of rdata fields enabled and set with an option to configure"?

[wtoorop]

I meant that it's millions, because of the return UINT_MAX; which is triggered by the raw_answer_producing_huge_memload.

Yes, that return is misleading as there is another restriction (rdlength) which would limit this. If we assume a minimum rdata field size of 1, anything larger than 65535 doesn't make sense anyway. I'll adapt that number to 65535.

[wtoorop]

What would be your proposal to "limit the number of rdata fields enabled and set with an option to configure"?

I'm proposing an option to configure to limit the maximum: --with-maximum-rdata-fields=256
I also think it would be better to have an alternative name for the function (for example ldns_maximum_rdata_fields) and refer to that from the function documentation, as ldns_rr_descriptors_maximum, has nothing to do with the maximum number of descriptors, but only says something about the maximum number of rdata fields (for a descriptor).

[wtoorop]

I meant that it's millions, because of the return UINT_MAX; which is triggered by the raw_answer_producing_huge_memload.

Yes, that return is misleading as there is another restriction (rdlength) which would limit this. If we assume a minimum rdata field size of 1, anything larger than 65535 doesn't make sense anyway. I'll adapt that number to 65535.

Done now in commit b7a7918 . I also took the opportunity to provide better names for the functions (ldns_minimum_rdata_fields and ldns_maximum_rdata_fields)

[xypiie]

What would be your proposal to "limit the number of rdata fields enabled and set with an option to configure"?

I'm proposing an option to configure to limit the maximum: --with-maximum-rdata-fields=256 I also think it would be better to have an alternative name for the function (for example ldns_maximum_rdata_fields) and refer to that from the function documentation, as ldns_rr_descriptors_maximum, has nothing to do with the maximum number of descriptors, but only says something about the maximum number of rdata fields (for a descriptor).

Ok, thanks a lot, please ping once you have the commit for this configure option and I'll retest and close this PR then.