From b91d9af5e34c1b0b9499b678cb457263a67dc332 Mon Sep 17 00:00:00 2001 From: Yihong Wu <54519668+domosekai@users.noreply.github.com> Date: Sun, 12 Dec 2021 17:00:03 +0800 Subject: [PATCH] Mayaqua/DNS: Fix memory safety in DNS operation threads Fix #1329 --- src/Mayaqua/DNS.c | 121 +++++++++++++++++++++++++++++------------- src/Mayaqua/DNS.h | 7 ++- src/Mayaqua/Network.c | 10 ++-- 3 files changed, 95 insertions(+), 43 deletions(-) diff --git a/src/Mayaqua/DNS.c b/src/Mayaqua/DNS.c index 9d871586..92b3ef63 100644 --- a/src/Mayaqua/DNS.c +++ b/src/Mayaqua/DNS.c @@ -318,10 +318,10 @@ bool DnsResolve(IP *ipv6, IP *ipv4, const char *hostname, UINT timeout, volatile return false; } - LIST *iplist_v6 = NewListFast(NULL); - LIST *iplist_v4 = NewListFast(NULL); + LIST *iplist_v6 = NULL; + LIST *iplist_v4 = NULL; - bool ret = DnsResolveEx(iplist_v6, iplist_v4, hostname, timeout, cancel_flag); + bool ret = DnsResolveEx(&iplist_v6, &iplist_v4, hostname, timeout, cancel_flag); if (ipv6 != NULL && LIST_NUM(iplist_v6) > 0) { @@ -348,7 +348,7 @@ bool DnsResolve(IP *ipv6, IP *ipv4, const char *hostname, UINT timeout, volatile return ret; } -bool DnsResolveEx(LIST *iplist_v6, LIST *iplist_v4, const char *hostname, UINT timeout, volatile const bool *cancel_flag) +bool DnsResolveEx(LIST **iplist_v6, LIST **iplist_v4, const char *hostname, UINT timeout, volatile const bool *cancel_flag) { if (iplist_v6 == NULL || iplist_v4 == NULL || IsEmptyStr(hostname)) { @@ -360,8 +360,10 @@ bool DnsResolveEx(LIST *iplist_v6, LIST *iplist_v4, const char *hostname, UINT t IP ipv6, ipv4; GetLocalHostIP6(&ipv6); GetLocalHostIP4(&ipv4); - AddHostIPAddressToList(iplist_v6, &ipv6); - AddHostIPAddressToList(iplist_v4, &ipv4); + *iplist_v6 = NewListFast(NULL); + *iplist_v4 = NewListFast(NULL); + AddHostIPAddressToList(*iplist_v6, &ipv6); + AddHostIPAddressToList(*iplist_v4, &ipv4); return true; } @@ -370,12 +372,14 @@ bool DnsResolveEx(LIST *iplist_v6, LIST *iplist_v4, const char *hostname, UINT t { if (IsIP6(&ip)) { - AddHostIPAddressToList(iplist_v6, &ip); + *iplist_v6 = NewListFast(NULL); + AddHostIPAddressToList(*iplist_v6, &ip); return true; } else { - AddHostIPAddressToList(iplist_v4, &ip); + *iplist_v4 = NewListFast(NULL); + AddHostIPAddressToList(*iplist_v4, &ip); return true; } @@ -402,13 +406,14 @@ bool DnsResolveEx(LIST *iplist_v6, LIST *iplist_v4, const char *hostname, UINT t Inc(threads_counter); - DNS_RESOLVER resolver; - Zero(&resolver, sizeof(resolver)); - resolver.IPList_v6 = iplist_v6; - resolver.IPList_v4 = iplist_v4; - resolver.Hostname = hostname; + DNS_RESOLVER *resolver; + resolver = ZeroMalloc(sizeof(DNS_RESOLVER)); + resolver->Ref = NewRef(); + resolver->IPList_v6 = NewListFast(NULL); + resolver->IPList_v4 = NewListFast(NULL); + resolver->Hostname = CopyStr(hostname); - THREAD *worker = NewThread(DnsResolver, &resolver); + THREAD *worker = NewThread(DnsResolver, resolver); WaitThreadInit(worker); if (cancel_flag == NULL) @@ -439,12 +444,20 @@ bool DnsResolveEx(LIST *iplist_v6, LIST *iplist_v4, const char *hostname, UINT t Dec(threads_counter); - if (resolver.OK) + if (resolver->OK) { - DnsCacheUpdateEx(hostname, iplist_v6, iplist_v4); + *iplist_v6 = resolver->IPList_v6; + *iplist_v4 = resolver->IPList_v4; + resolver->IPList_v6 = NULL; + resolver->IPList_v4 = NULL; + DnsCacheUpdateEx(hostname, *iplist_v6, *iplist_v4); + ReleaseDnsResolver(resolver); return true; } + + ReleaseDnsResolver(resolver); + CACHE: Debug("DnsResolve(): Could not resolve \"%s\". Searching for it in the cache...\n", hostname); @@ -454,19 +467,8 @@ CACHE: return false; } - UINT i; - - for (i = 0; i < LIST_NUM(cached->IPList_v6); ++i) - { - IP *ip = LIST_DATA(cached->IPList_v6, i); - AddHostIPAddressToList(iplist_v6, ip); - } - - for (i = 0; i < LIST_NUM(cached->IPList_v4); ++i) - { - IP *ip = LIST_DATA(cached->IPList_v4, i); - AddHostIPAddressToList(iplist_v4, ip); - } + *iplist_v6 = CloneIPAddressList(cached->IPList_v6); + *iplist_v4 = CloneIPAddressList(cached->IPList_v4); return true; } @@ -480,6 +482,8 @@ void DnsResolver(THREAD *t, void *param) DNS_RESOLVER *resolver = param; + AddRef(resolver->Ref); + NoticeThreadInit(t); AddWaitThread(t); @@ -542,6 +546,8 @@ void DnsResolver(THREAD *t, void *param) Debug("DnsResolver(): getaddrinfo() failed with error %d!\n", ret); } + ReleaseDnsResolver(resolver); + DelWaitThread(t); } @@ -572,11 +578,12 @@ bool DnsResolveReverse(char *dst, const UINT size, const IP *ip, UINT timeout, v Inc(threads_counter); - DNS_RESOLVER_REVERSE resolver; - Zero(&resolver, sizeof(resolver)); - Copy(&resolver.IP, ip, sizeof(resolver.IP)); + DNS_RESOLVER_REVERSE *resolver; + resolver = ZeroMalloc(sizeof(DNS_RESOLVER_REVERSE)); + resolver->Ref = NewRef(); + Copy(&resolver->IP, ip, sizeof(resolver->IP)); - THREAD *worker = NewThread(DnsResolverReverse, &resolver); + THREAD *worker = NewThread(DnsResolverReverse, resolver); WaitThreadInit(worker); if (cancel_flag == NULL) @@ -607,15 +614,17 @@ bool DnsResolveReverse(char *dst, const UINT size, const IP *ip, UINT timeout, v Dec(threads_counter); - if (resolver.OK) + if (resolver->OK) { - StrCpy(dst, size, resolver.Hostname); - Free(resolver.Hostname); - + StrCpy(dst, size, resolver->Hostname); DnsCacheReverseUpdate(ip, dst); + ReleaseDnsResolverReverse(resolver); return true; } + + ReleaseDnsResolverReverse(resolver); + CACHE: Debug("DnsResolveReverse(): Could not resolve \"%r\". Searching for it in the cache...\n", ip); @@ -639,6 +648,8 @@ void DnsResolverReverse(THREAD *t, void *param) DNS_RESOLVER_REVERSE *resolver = param; + AddRef(resolver->Ref); + NoticeThreadInit(t); AddWaitThread(t); @@ -659,6 +670,8 @@ void DnsResolverReverse(THREAD *t, void *param) Debug("DnsResolverReverse(): getnameinfo() failed with error %d!\n", ret); } + ReleaseDnsResolverReverse(resolver); + DelWaitThread(t); } @@ -688,3 +701,37 @@ bool GetIPEx(IP *ip, const char *hostname, UINT timeout, volatile const bool *ca return false; } + +// Release of the parameters of the DNS Resolver thread +void ReleaseDnsResolver(DNS_RESOLVER *p) +{ + // Validate arguments + if (p == NULL) + { + return; + } + + if (Release(p->Ref) == 0) + { + FreeHostIPAddressList(p->IPList_v6); + FreeHostIPAddressList(p->IPList_v4); + Free(p->Hostname); + Free(p); + } +} + +// Release of the parameters of the DNS Resolver Reverse thread +void ReleaseDnsResolverReverse(DNS_RESOLVER_REVERSE *p) +{ + // Validate arguments + if (p == NULL) + { + return; + } + + if (Release(p->Ref) == 0) + { + Free(p->Hostname); + Free(p); + } +} \ No newline at end of file diff --git a/src/Mayaqua/DNS.h b/src/Mayaqua/DNS.h index 98e5f426..462c8c43 100644 --- a/src/Mayaqua/DNS.h +++ b/src/Mayaqua/DNS.h @@ -38,6 +38,7 @@ struct DNS_CACHE_REVERSE struct DNS_RESOLVER { + REF *Ref; const char *Hostname; LIST *IPList_v4; LIST *IPList_v6; @@ -46,6 +47,7 @@ struct DNS_RESOLVER struct DNS_RESOLVER_REVERSE { + REF *Ref; IP IP; char *Hostname; bool OK; @@ -69,7 +71,7 @@ DNS_CACHE_REVERSE *DnsCacheReverseFind(const IP *ip); DNS_CACHE_REVERSE *DnsCacheReverseUpdate(const IP *ip, const char *hostname); bool DnsResolve(IP *ipv6, IP *ipv4, const char *hostname, UINT timeout, volatile const bool *cancel_flag); -bool DnsResolveEx(LIST *iplist_v6, LIST *iplist_v4, const char *hostname, UINT timeout, volatile const bool *cancel_flag); +bool DnsResolveEx(LIST **iplist_v6, LIST **iplist_v4, const char *hostname, UINT timeout, volatile const bool *cancel_flag); void DnsResolver(THREAD *t, void *param); bool DnsResolveReverse(char *dst, const UINT size, const IP *ip, UINT timeout, volatile const bool *cancel_flag); @@ -77,4 +79,7 @@ void DnsResolverReverse(THREAD *t, void *param); bool GetIPEx(IP *ip, const char *hostname, UINT timeout, volatile const bool *cancel_flag); +void ReleaseDnsResolver(DNS_RESOLVER *p); +void ReleaseDnsResolverReverse(DNS_RESOLVER_REVERSE *p); + #endif diff --git a/src/Mayaqua/Network.c b/src/Mayaqua/Network.c index 38769b0e..ee97595d 100644 --- a/src/Mayaqua/Network.c +++ b/src/Mayaqua/Network.c @@ -14484,18 +14484,20 @@ SOCK *ConnectEx4(char *hostname, UINT port, UINT timeout, bool *cancel_flag, cha StrCpy(hostname_original, sizeof(hostname_original), hostname); } - LIST *iplist_v6 = NewListFast(NULL); - LIST *iplist_v4 = NewListFast(NULL); + LIST *iplist_v6 = NULL; + LIST *iplist_v4 = NULL; if (IsZeroIp(ret_ip) == false) { // Skip name resolution if (IsIP6(ret_ip)) { + iplist_v6 = NewListFast(NULL); AddHostIPAddressToList(iplist_v6, ret_ip); } else { + iplist_v4 = NewListFast(NULL); AddHostIPAddressToList(iplist_v4, ret_ip); } @@ -14504,10 +14506,8 @@ SOCK *ConnectEx4(char *hostname, UINT port, UINT timeout, bool *cancel_flag, cha else { // Forward resolution - if (DnsResolveEx(iplist_v6, iplist_v4, hostname_original, 0, cancel_flag) == false) + if (DnsResolveEx(&iplist_v6, &iplist_v4, hostname_original, 0, cancel_flag) == false) { - FreeHostIPAddressList(iplist_v6); - FreeHostIPAddressList(iplist_v4); return NULL; } }