tinc 1.1: freeaddrinfo(NULL) crash on windows

[Guus Sliepen]

On Fri, Feb 16, 2018 at 02:26:36PM -0700, Todd C. Miller wrote:

> > Thanks for reporting this bug! Indeed, that code is wrong. It should
> > free cache->ai instead of cache->aip. Even if it didn't crash on Linux,
> > it was a memory leak. A fix is now in the git repository.
> 
> The fix leads to a use after free bug which is visible on macOS.
> It doesn't seem to be much of a problem on most platforms since a
> copy of freed memory is made almost immediately but on macOS the
> memory appears to be cleared much of the time.  This results in an
> attempt to connect to an AF_UNSPEC socket with no address, which
> of course fails.

Reading freed memory is always a bug.

> A potential fix, modeled on the "recently seen addresses not in our
> cache" part of get_recent_address() is:
> 
>     https://github.com/gsliepen/tinc/pull/177

Thanks again for providing a patch!

> Another approach would be to pass the sockaddr_t to get_recent_address()
> and fill it in there.  I can write a diff for that if you'd like.

The fix in the pull request looks fine, I don't think another approach
is better.

I'll have to invest some time in improving the test suite, and using
code coverage checking to ensure the test suite covers all everything
(if that's possible). Then the test suite can be run with
valgrind/asan/ubsan/etc. to prevent bugs like this from being committed
in the first place.

-- 
Met vriendelijke groet / with kind regards,
     Guus Sliepen <guus at tinc-vpn.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://www.tinc-vpn.org/pipermail/tinc-devel/attachments/20180217/5ebb5dde/attachment.sig>