Make DNS lookup mockable/testable/fuzzable.
Add fuzzing harness for Lookup(…)/LookupHost(…)/LookupNumeric(…)/LookupSubNet(…).
See doc/fuzzing.md for information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the Bitcoin Core fuzzing corpus repo.
Happy fuzzing :)
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--174a7506f384e20aa4161008e828411d-->
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
Concept ACK
82 | - vIP.push_back(addr); 83 | - return true; 84 | - } 85 | - } 86 | - 87 | +static Optional<std::vector<CNetAddr>> LookupInternGetAddrInfo(const std::string& name, bool fAllowLookup) {
This is changed in bfc1455 to WrappedGetAddrInfo, why not do that here?
I could have done it already in the earlier commit. I suggest keeping the current commit order in not invalidate the ACK. Makes sense? :)
Thanks a lot for reviewing BTW ❤️
Yeah that makes sense. Fuzzing is a labor of love
Tested ACK 426617a. Coverage for ~48 hours of libfuzzer fuzzing here: https://crypt-iq.github.io/netbase_dns_cov_outs/src/netbase.cpp.gcov.html
Concept ACK
ACK 954fbb74971a9056043f6f98e09fb8f175244031
Code change is straightforward to reason about. Ubuntu 18:
./configure --enable-fuzz --with-sanitizers=address,undefined,integer,fuzzer reports no errorsmacOS v10.15.4:
./configure --enable-fuzz --with-sanitizers=address,fuzzer --disable-asm reports no errors./configure --enable-fuzz --with-sanitizers=undefined,integer,fuzzer reports the following:<details> <summary>implicit-signed-integer-truncation</summary>
netbase.cpp:240:37: runtime error: implicit conversion from type 'int' of value -1877995509 (32-bit, signed) to type 'uint16_t' (aka 'unsigned short') changed the value to 4107 (16-bit, unsigned)
[#0](/bitcoin-bitcoin/0/) 0x109a6b73b in Lookup(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::vector<CService, std::__1::allocator<CService> >&, int, bool, unsigned int, std::__1::function<boost::optional<std::__1::vector<CNetAddr, std::__1::allocator<CNetAddr> > > (std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, bool)>)+0x7ab (netbase_dns_lookup:x86_64+0x101c0273b)
[#1](/bitcoin-bitcoin/1/) 0x109a6cad8 in Lookup(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, CService&, int, bool, std::__1::function<boost::optional<std::__1::vector<CNetAddr, std::__1::allocator<CNetAddr> > > (std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, bool)>)+0x338 (netbase_dns_lookup:x86_64+0x101c03ad8)
[#2](/bitcoin-bitcoin/2/) 0x107e73228 in test_one_input(std::__1::vector<unsigned char, std::__1::allocator<unsigned char> > const&)+0x5d8 (netbase_dns_lookup:x86_64+0x10000a228)
[#3](/bitcoin-bitcoin/3/) 0x10a304841 in LLVMFuzzerTestOneInput+0x151 (netbase_dns_lookup:x86_64+0x10249b841)
[#4](/bitcoin-bitcoin/4/) 0x10a66fac0 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) FuzzerLoop.cpp:556
[#5](/bitcoin-bitcoin/5/) 0x10a66f205 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool*) FuzzerLoop.cpp:470
[#6](/bitcoin-bitcoin/6/) 0x10a6718a7 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::__1::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) FuzzerLoop.cpp:765
[#7](/bitcoin-bitcoin/7/) 0x10a671c09 in fuzzer::Fuzzer::Loop(std::__1::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) FuzzerLoop.cpp:792
[#8](/bitcoin-bitcoin/8/) 0x10a65f3ed in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) FuzzerDriver.cpp:829
[#9](/bitcoin-bitcoin/9/) 0x10a68a0b2 in main FuzzerMain.cpp:19
[#10](/bitcoin-bitcoin/10/) 0x7fff73778cc8 in start+0x0 (libdyld.dylib:x86_64+0x1acc8)
SUMMARY: UndefinedBehaviorSanitizer: implicit-signed-integer-truncation netbase.cpp:240:37 in
</details>
Had to rebase after the merge of 15c27c44417ab77a660b53b8574f7eb5261b19f8.
Changed:
diff --git a/src/Makefile.test.include b/src/Makefile.test.include
index 73eb9ad56..d9bf704bd 100644
--- a/src/Makefile.test.include
+++ b/src/Makefile.test.include
@@ -755,7 +755,7 @@ test_fuzz_netaddress_SOURCES = test/fuzz/netaddress.cpp
test_fuzz_netbase_dns_lookup_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES)
test_fuzz_netbase_dns_lookup_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
test_fuzz_netbase_dns_lookup_LDADD = $(FUZZ_SUITE_LD_COMMON)
-test_fuzz_netbase_dns_lookup_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS)
+test_fuzz_netbase_dns_lookup_LDFLAGS = $(FUZZ_SUITE_LDFLAGS_COMMON)
test_fuzz_netbase_dns_lookup_SOURCES = test/fuzz/netbase_dns_lookup.cpp
test_fuzz_out_point_deserialize_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) -DOUT_POINT_DESERIALIZE=1
@Crypt-iQ Thanks a lot for reviewing. May I ask you a re-review and hopefully give your third Tested ACK for this change? :)
ACK 6f9bc3f
221 | @@ -204,7 +222,7 @@ bool LookupHost(const std::string& name, CNetAddr& addr, bool fAllowLookup) 222 | * @returns Whether or not the service string successfully resolved to any 223 | * resulting services. 224 | */ 225 | -bool Lookup(const std::string& name, std::vector<CService>& vAddr, int portDefault, bool fAllowLookup, unsigned int nMaxSolutions) 226 | +bool Lookup(const std::string& name, std::vector<CService>& vAddr, int portDefault, bool fAllowLookup, unsigned int nMaxSolutions, std::function<Optional<std::vector<CNetAddr>>(const std::string&, bool)> mockable_dns_lookup_function)
camelCase is used for the other arguments, is it fine to have an odd snake case argument?
Yes, for new code it's preferred. See doc/developer-notes.md, "Coding Style (C++), Symbol naming conventions".
6f9bc3f67bd177cc7a5dc4f93d0c20c2d23307ec
No issue found when fuzzing with ./configure --enable-fuzz --with-sanitizers=address,fuzzer CC=/usr/local/opt/llvm/bin/clang CXX=/usr/local/opt/llvm/bin/clang++ --disable-asm for 6 hours.
I tried to run the fuzz on mac os with
./configure --enable-fuzz --with-sanitizers=fuzzer,address,undefined CC=/usr/local/opt/llvm/bin/clang CXX=/usr/local/opt/llvm/bin/clang++ --disable-asm
<details><summary>But it fails (I guess it's an issue with my setup)</summary>
▶ ./src/test/fuzz/process_message
/usr/local/opt/llvm/bin/../include/c++/v1/string:2728:44: runtime error: member call on misaligned address 0x000000000001 for type 'std::__1::basic_string<char> *', which requires 8 byte alignment
0x000000000001: note: pointer points here
<memory cannot be printed>
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/local/opt/llvm/bin/../include/c++/v1/string:2728:44 in
AddressSanitizer:DEADLYSIGNAL
=================================================================
==4275==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000001 (pc 0x7fff6c3d7712 bp 0x7ffeef323bd0 sp 0x7ffeef323ba0 T0)
==4275==The signal is caused by a READ memory access.
==4275==Hint: address points to the zero page.
[#0](/bitcoin-bitcoin/0/) 0x7fff6c3d7712 in std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::insert(unsigned long, char const*, unsigned long)+0x1a (libc++.1.dylib:x86_64+0x38712)
[#1](/bitcoin-bitcoin/1/) 0x101cc2d89 in std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::insert(unsigned long, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) string:2730
[#2](/bitcoin-bitcoin/2/) 0x101c3c557 in BCLog::Logger::LogPrintStr(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) logging.cpp:245
[#3](/bitcoin-bitcoin/3/) 0x100ac6bee in void LogPrintf<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >(char const*, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) logging.h:176
[#4](/bitcoin-bitcoin/4/) 0x100ac63c0 in InitLogging(ArgsManager const&) init.cpp:869
[#5](/bitcoin-bitcoin/5/) 0x101da7053 in BasicTestingSetup::BasicTestingSetup(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::vector<char const*, std::__1::allocator<char const*> > const&) setup_common.cpp:100
[#6](/bitcoin-bitcoin/6/) 0x101da941f in TestingSetup::TestingSetup(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::vector<char const*, std::__1::allocator<char const*> > const&) setup_common.cpp:128
[#7](/bitcoin-bitcoin/7/) 0x1008dd4e4 in initialize() process_message.cpp:48
[#8](/bitcoin-bitcoin/8/) 0x101e2f77c in LLVMFuzzerInitialize fuzz.cpp:52
[#9](/bitcoin-bitcoin/9/) 0x102072f87 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) FuzzerDriver.cpp:616
[#10](/bitcoin-bitcoin/10/) 0x1020a08e2 in main FuzzerMain.cpp:19
[#11](/bitcoin-bitcoin/11/) 0x7fff6f0d4cc8 in start+0x0 (libdyld.dylib:x86_64+0x1acc8)
==4275==Register values:
rax = 0x0000000000000007 rbx = 0x0000000000000000 rcx = 0x0000000000000007 rdx = 0x0000000103eb3081
rdi = 0x0000000000000001 rsi = 0x0000000000000000 rbp = 0x00007ffeef323bd0 rsp = 0x00007ffeef323ba0
r8 = 0x0000000103eb3000 r9 = 0x0000008047835f00 r10 = 0x00007fff6f218a46 r11 = 0x0000000000000206
r12 = 0x0000000103eb3081 r13 = 0x0000000000000000 r14 = 0x0000000000000007 r15 = 0x0000000000000001
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (libc++.1.dylib:x86_64+0x38712) in std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::insert(unsigned long, char const*, unsigned long)+0x1a
==4275==ABORTING
[1] 4275 abort ./src/test/fuzz/process_message
</details>
@D4nte for macOS builds, specifying the address and undefined sanitizers won't work. This is tracked here https://github.com/bitcoin/bitcoin/issues/19789
@D4nte for macOS builds, specifying the
addressandundefinedsanitizers won't work. This is tracked here #19789
Thanks. Ran with ./configure --enable-fuzz --with-sanitizers=fuzzer,undefined CC=/usr/local/opt/llvm/bin/clang CXX=/usr/local/opt/llvm/bin/clang++ --disable-asm for 6 hours no issue encountered.
@D4nte for macOS builds, specifying the
addressandundefinedsanitizers won't work. This is tracked here #19789Thanks. Ran with
./configure --enable-fuzz --with-sanitizers=fuzzer,undefined CC=/usr/local/opt/llvm/bin/clang CXX=/usr/local/opt/llvm/bin/clang++ --disable-asmfor 6 hours no issue encountered.
Keep your seeds! We should PR them to bitcoin-core/qa-assets after this is merged.
@D4nte for macOS builds, specifying the
addressandundefinedsanitizers won't work. This is tracked here #19789Thanks. Ran with
./configure --enable-fuzz --with-sanitizers=fuzzer,undefined CC=/usr/local/opt/llvm/bin/clang CXX=/usr/local/opt/llvm/bin/clang++ --disable-asmfor 6 hours no issue encountered.Keep your seeds! We should PR them to bitcoin-core/qa-assets after this is merged.
Ok thanks, I did not delete anything. I'll prepare a branch/fork to PR them.
Updated: Added more assertions to the fuzzer to test relevant postconditions. Simplified the code that allows for mocking.
69 | + // IPv4,v6 addresses to vIP while respecting nMaxSolutions. 70 | + struct addrinfo *aiTrav = aiRes; 71 | + std::vector<CNetAddr> resolved_addresses; 72 | + while (aiTrav != nullptr) 73 | + { 74 | + CNetAddr resolved;
this is unused
Thanks! Fixed!
0 | @@ -0,0 +1,77 @@ 1 | +// Copyright (c) 2020 The Bitcoin Core developers 2 | +// Distributed under the MIT software license, see the accompanying 3 | +// file COPYING or http://www.opensource.org/licenses/mit-license.php. 4 | + 5 | +#include <netbase.h> 6 | +#include <optional.h>
AFAICT optional.h isn't used
Thanks! Fixed!
0 | @@ -0,0 +1,77 @@ 1 | +// Copyright (c) 2020 The Bitcoin Core developers
The year has updated 🎇
Thanks! Fixed!
Two more comments, running this patch.
@Crypt-iQ Thanks for reviewing. I believe all feedback should be addressed now: mind re-reviewing one last time (hopefully)? :)
I get the following complaint when running --with-sanitizers=integer because CService expects uint16_t
netbase.cpp:240:37: runtime error: implicit conversion from type 'int' of value 210314121 (32-bit, signed) to type 'uint16_t' (aka 'unsigned short') changed the value to 9097 (16-bit, unsigned)
Tested ACK 0f2cfb379d8a04efc15d14ee581d3d1ad8ccfd97
63 | + 64 | + struct addrinfo *aiRes = nullptr; 65 | + int nErr = getaddrinfo(name.c_str(), nullptr, &aiHint, &aiRes); 66 | + if (nErr) 67 | + { 68 | + return {};
nit: opening { on the same line as if. Or, given that nErr is not used elsewhere maybe:
if (getaddrinfo(name.c_str(), nullptr, &aiHint, &aiRes) != 0) {
return {};
78 | + { 79 | + assert(aiTrav->ai_addrlen >= sizeof(sockaddr_in)); 80 | + resolved_addresses.emplace_back(((struct sockaddr_in*)(aiTrav->ai_addr))->sin_addr); 81 | + } 82 | + if (aiTrav->ai_family == AF_INET6) 83 | + {
nits: this is not move-only change, so maybe worth fixing the style, opening { on the same line.
183 | + for (const CNetAddr& resolved : dns_lookup_function(name, fAllowLookup)) 184 | { 185 | - CNetAddr resolved; 186 | - if (aiTrav->ai_family == AF_INET) 187 | + if (nMaxSolutions == 0 || vIP.size() < nMaxSolutions) 188 | {
nit: opening { on the same line
178 | - 179 | - // Traverse the linked list starting with aiTrav, add all non-internal 180 | - // IPv4,v6 addresses to vIP while respecting nMaxSolutions. 181 | - struct addrinfo *aiTrav = aiRes; 182 | - while (aiTrav != nullptr && (nMaxSolutions == 0 || vIP.size() < nMaxSolutions)) 183 | + for (const CNetAddr& resolved : dns_lookup_function(name, fAllowLookup))
We can quit from this loop earlier if we got enough addresses:
for (const CNetAddr& resolved : dns_lookup_function(name, fAllowLookup)) {
if (nMaxSolutions > 0 && vIP.size() >= nMaxSolutions) {
break;
}
/* Never allow resolving to an internal address. Consider any such result invalid */
if (!resolved.IsInternal()) {
vIP.push_back(resolved);
}
}
Now updated as suggested.
63 | + } 64 | + } 65 | + { 66 | + CService resolved_service = LookupNumeric(name, default_port, fuzzed_dns_lookup_function); 67 | + assert(!resolved_service.IsInternal()); 68 | + }
nit: here and elsewhere in the test, those extra { and } are not necessary.
They are intentional scope delimiters :)
ACK 522b3b4bc276378f93f1b6109be9c66a7f8bfd38
Some nits below.
Ran this for some time and submitted the generated seeds at https://github.com/bitcoin-core/qa-assets/pull/50.
60 | -bool Lookup(const std::string& name, CService& addr, int portDefault, bool fAllowLookup); 61 | -bool Lookup(const std::string& name, std::vector<CService>& vAddr, int portDefault, bool fAllowLookup, unsigned int nMaxSolutions); 62 | -CService LookupNumeric(const std::string& name, int portDefault = 0); 63 | -bool LookupSubNet(const std::string& strSubnet, CSubNet& subnet); 64 | + 65 | +using DNSLookupFn = std::function<std::vector<CNetAddr>(const std::string&, bool)>;
This is good as it is. Maybe it would be a bit more convenient to have a single global variable of this type (resolver function) which defaults to WrappedGetAddrInfo() and is changed in the tests. Something like:
DNSLookupFn g_dns_lookup = WrappedGetAddrInfo;
bool LookupHost(..., DNSLookupFn dns_lookup_function = g_dns_lookup);
// in the tests
g_dns_lookup = fuzzed_dns_lookup_function;
Actually, there is more to this: using such a global would allow testing methods that call Lookup*() themselves. In such cases it would not be possible for the test to override the dns resolver by calling Lookup*() with a non-default argument.
For example CConnman::ConnectNode() calls Lookup() without passing a non-default resolver to it (of course). If we use g_dns_lookup, then a test could modify its value to point to a fuzzed resolver and then call CConnman::ConnectNode().
Good points. Now using g_dns_lookup as suggested.
ACK 24e3ce3d51ea0ff7f19519f08db18e8df693473d
Concept ACK, will review.
I get the following complaint when running
--with-sanitizers=integerbecauseCServiceexpectsuint16_tnetbase.cpp:240:37: runtime error: implicit conversion from type 'int' of value 210314121 (32-bit, signed) to type 'uint16_t' (aka 'unsigned short') changed the value to 9097 (16-bit, unsigned)https://github.com/bitcoin/bitcoin/blob/1a22951f545b549b8b9a9266dc61a740a69d4dbe/src/netbase.cpp#L240 @Crypt-iQ Good find. This should be fixed with #21328.
65 | + if (nErr) { 66 | + return {}; 67 | + } 68 | + 69 | + // Traverse the linked list starting with aiTrav, add all non-internal 70 | + // IPv4,v6 addresses to vIP while respecting nMaxSolutions.
This comment could be clarified - vIP + nMaxSolutions aren't used here
Done!
16 | + 17 | +std::vector<CNetAddr> fuzzed_dns_lookup_function(const std::string& name, bool allow_lookup) 18 | +{ 19 | + std::vector<CNetAddr> resolved_addresses; 20 | + while (fuzzed_data_provider_ptr->ConsumeBool()) { 21 | + resolved_addresses.push_back(ConsumeNetAddr(*fuzzed_data_provider_ptr));
After reading the getaddrinfo manpage, I realized ConsumeNetAddr can return CNetAddr with m_scope_id set for non link-local addresses, which is different from getaddrinfo behaviour. I'm not an expert here but I think there could be sanity-checking for that in ConsumeNetAddr.
I don't think that is likely to matter in practice (don't hesitate to prove me wrong!). Leaving unchanged for now.
Some comments, will test again
60 | + // hostname lookups. 61 | + aiHint.ai_flags = allow_lookup ? AI_ADDRCONFIG : AI_NUMERICHOST; 62 | + 63 | + struct addrinfo* aiRes = nullptr; 64 | + int nErr = getaddrinfo(name.c_str(), nullptr, &aiHint, &aiRes); 65 | + if (nErr) {
proposed comment (and maybe simplification, as we're not using nErr for anything, otherwise if we keep nErr it can be const)
- int nErr = getaddrinfo(name.c_str(), nullptr, &aiHint, &aiRes);
- if (nErr) {
+ // getaddrinfo(3) returns 0 if it succeeds or a non-zero error code.
+ if (getaddrinfo(name.c_str(), nullptr, &aiHint, &aiRes)) {
Made n_err const as suggested. Now using the more explicit if (n_err != 0) which I find easier to read.
75 | + assert(aiTrav->ai_addrlen >= sizeof(sockaddr_in)); 76 | + resolved_addresses.emplace_back(((struct sockaddr_in*)(aiTrav->ai_addr))->sin_addr); 77 | + } 78 | + if (aiTrav->ai_family == AF_INET6) { 79 | + assert(aiTrav->ai_addrlen >= sizeof(sockaddr_in6)); 80 | + struct sockaddr_in6* s6 = (struct sockaddr_in6*)aiTrav->ai_addr;
here and line 76, can we use named casts?
Sure. Now using reinterpret_cast.
85 | + freeaddrinfo(aiRes); 86 | + 87 | + return resolved_addresses; 88 | +} 89 | + 90 | +DNSLookupFn g_dns_lookup = WrappedGetAddrInfo;
DNSLookupFn g_dns_lookup{WrappedGetAddrInfo};
58 | + // If we don't allow lookups, then use the AI_NUMERICHOST flag for 59 | + // getaddrinfo to only decode numerical network addresses and suppress 60 | + // hostname lookups. 61 | + aiHint.ai_flags = allow_lookup ? AI_ADDRCONFIG : AI_NUMERICHOST; 62 | + 63 | + struct addrinfo* aiRes = nullptr;
struct addrinfo* aiRes{nullptr};
66 | + return {}; 67 | + } 68 | + 69 | + // Traverse the linked list starting with aiTrav, add all non-internal 70 | + // IPv4,v6 addresses to vIP while respecting nMaxSolutions. 71 | + struct addrinfo* aiTrav = aiRes;
struct addrinfo* aiTrav{aiRes};
193 | - assert(aiTrav->ai_addrlen >= sizeof(sockaddr_in6)); 194 | - struct sockaddr_in6* s6 = (struct sockaddr_in6*) aiTrav->ai_addr; 195 | - resolved = CNetAddr(s6->sin6_addr, s6->sin6_scope_id); 196 | + for (const CNetAddr& resolved : dns_lookup_function(name, fAllowLookup)) { 197 | + if (nMaxSolutions > 0 && vIP.size() >= nMaxSolutions) { 198 | + break;
Can this if (nMaxSolutions > 0 && vIP.size() >= nMaxSolutions) check be done before the loop?
Not that I can see: vIP is added to within the loop.
Please provide a diff explaining your suggestion if it still stands :)
You're right, thanks.
0 | @@ -0,0 +1,76 @@ 1 | +// Copyright (c) 2021 The Bitcoin Core developers 2 | +// Distributed under the MIT software license, see the accompanying 3 | +// file COPYING or http://www.opensource.org/licenses/mit-license.php. 4 | + 5 | +#include <netbase.h>
suggestion, as this uses CNetAddr defined in netaddress.h
+#include <netaddress.h>
Thanks! Addressed.
Tested approach ACK, a few comments below.
$ FUZZ=netbase_dns_lookup src/test/fuzz/fuzz ../qa-assets/fuzz_seed_corpus/
INFO: Seed: 3479425365
INFO: Loaded 1 modules (643152 inline 8-bit counters): 643152 [0x555d231dc188, 0x555d232791d8),
INFO: Loaded 1 PC tables (643152 PCs): 643152 [0x555d232791d8,0x555d23c496d8),
INFO: 129774 files found in ../qa-assets/fuzz_seed_corpus/
INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 1048576 bytes
INFO: seed corpus: files: 129774 min: 1b max: 3986616b total: 1875939121b rss: 231Mb
[#8192](/bitcoin-bitcoin/8192/) pulse cov: 970 ft: 1825 corp: 59/244b exec/s: 4096 rss: 337Mb
[#16384](/bitcoin-bitcoin/16384/) pulse cov: 1763 ft: 3612 corp: 98/809b exec/s: 4096 rss: 436Mb
[#32768](/bitcoin-bitcoin/32768/) pulse cov: 2090 ft: 6274 corp: 158/2458b exec/s: 4681 rss: 590Mb
[#65536](/bitcoin-bitcoin/65536/) pulse cov: 2121 ft: 7423 corp: 192/5608b exec/s: 5041 rss: 593Mb
[#129775](/bitcoin-bitcoin/129775/) INITED cov: 2235 ft: 10320 corp: 297/984Kb exec/s: 3415 rss: 1271Mb
[#129813](/bitcoin-bitcoin/129813/) REDUCE cov: 2235 ft: 10320 corp: 297/984Kb lim: 286065 exec/s: 3416 rss: 1271Mb L: 62/286047 MS: 3 ChangeBit-ChangeBit-EraseBytes-
.../...
[#113401432](/bitcoin-bitcoin/113401432/) REDUCE cov: 2669 ft: 12318 corp: 458/295Kb lim: 1048576 exec/s: 1595 rss: 1271Mb L: 942/16122 MS: 4 CMP-ChangeBit-ChangeBit-EraseBytes- DE: "\x9e\xff\xff\xff"-
[#113449634](/bitcoin-bitcoin/113449634/) REDUCE cov: 2669 ft: 12318 corp: 458/295Kb lim: 1048576 exec/s: 1595 rss: 1271Mb L: 2682/16122 MS: 2 ShuffleBytes-EraseBytes-
[#113480169](/bitcoin-bitcoin/113480169/) REDUCE cov: 2669 ft: 12318 corp: 458/295Kb lim: 1048576 exec/s: 1595 rss: 1271Mb L: 7438/16122 MS: 5 ChangeBit-EraseBytes-PersAutoDict-InsertRepeatedBytes-InsertRepeatedBytes- DE: "\x00\x00bP\x01_\x86\xac"-
cr ACK e5280751890b02abb558b37eb0e0401493f148b4
ACK e5280751890b02abb558b37eb0e0401493f148b4
11 | +#include <cstdint> 12 | +#include <string> 13 | +#include <vector> 14 | + 15 | +namespace { 16 | +FuzzedDataProvider* fuzzed_data_provider_ptr = nullptr;
style-nit: Any reason for this namespace and pointer? Looks like a lambda should be able to do the same in less code and smaller scope
Using a lambda here is a good idea: less code, smaller scope and overall beautiful :)
TBH I don't remember why I didn't do it that way nine months ago when this code was written: a lambda is strictly better AFAICT :)
Address in PR #21443.
ACK e528075
Thanks for the update and s/aiTrav/ai_trav/ was a nice touchup.