Buffer overflow in function _evdns_nameserver_add_impl

[BushraAloraini]

Buffer overflow in function _evdns_nameserver_add_impl due to lack of validation before memcpy. This happens because there is no validation that addrlen matches address length.
In function evdns_base_nameserver_sockaddr_add, evdns_nameserver_add_impl will be called with values(sa and len) that might not match, meaning that the len might be more than sa, which will cause buffer error or even less than 0.

To reproduce:

• Build libevent with ASAN and then copy the attached file be.c to the libevent source directory and compile be.c with ASAN:

$ gcc -fsanitize=address -fomit-frame-pointer be.c build/lib/libevent.a -Iinclude -Ibuild/include -o be

=================================================================
==2404==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffec7b9b1d0 at pc 0x7f458f825935 bp 0x7ffec7b9b070 sp 0x7ffec7b9a818
READ of size 128 at 0x7ffec7b9b1d0 thread T0
    #0 0x7f458f825934 in __asan_memcpy (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x8c934)
    #1 0x4194c1 in evdns_nameserver_add_impl_.part.19 (/home/bushra/libevent/be1+0x4194c1)

This happens because slen exceeds the size of ss.

evdns_base_nameserver_sockaddr_add(evdns_base, (struct sockaddr*)&ss, slen, 0);

be.c.txt

[azat]

Thank you for the detailed report!

Buffer overflow in function _evdns_nameserver_add_impl due to lack of validation before memcpy

But what the validation should be?

In function evdns_base_nameserver_sockaddr_add, evdns_nameserver_add_impl will be called with values(sa and len) that might not match, meaning that the len might be more than sa, which will cause buffer error or even less than 0.

That said that, this is not the API responsibility to check that sizeof(*get_sockaddr_struct_from_type(sa->sa_family)) == addrlen, since then you will be limited to the types, that you explicitly support (although this is not a problem here, but I did not met such code).

[BushraAloraini]

In line 3230, there is a check to ensure that (addrlen) does not exceed the size of the destination (ns->address) to prevent buffer over-write. You can add another check to ensure that (addrlen) does not exceed the size of the source (address) to prevent buffer over-read or other memory corruption errors.

Thanks!

[azat]

It is not possible to add such a check since it accepts const struct sockaddr *sa and it cannot the underlying size.