Merge #18512: Improve asmap checks and add sanity check

748977690e Add asmap_direct fuzzer that tests Interpreter directly (Pieter Wuille)
7cf97fda15 Make asmap Interpreter errors fatal and fuzz test it (Pieter Wuille)
c81aefc537 Add additional effiency checks to sanity checker (Pieter Wuille)
fffd8dca2d Add asmap sanity checker (Pieter Wuille)
5feefbe6e7 Improve asmap Interpret checks and document failures (Pieter Wuille)
2b3dbfa5a6 Deal with decoding failures explicitly in asmap Interpret (Pieter Wuille)
1479007a33 Introduce Instruction enum in asmap (Pieter Wuille)

Pull request description:

  This improves/documents the failure cases inside the asmap interpreter. None of the changes are bug fixes (they only change behavior for corrupted asmap files), but they may make things easier to follow.

  In a second step, a sanity checker is added that effectively executes every potential code path through the asmap file, checking the same failure cases as the interpreter, and more. It takes around 30 ms to run for me for a 1.2 MB asmap file.

  I've verified that this accepts asmap files constructed by https://github.com/sipa/asmap/blob/master/buildmap.py with a large dataset, and no longer accepts it with 1 bit changed in it.

ACKs for top commit:
  practicalswift:
    ACK 748977690e modulo feedback below.
  jonatack:
    ACK 748977690e code review, regular build/tests/ran bitcoin with -asmap, fuzz build/ran both fuzzers overnight.
  fjahr:
    ACK 748977690e

Tree-SHA512: d876df3859735795c857c83e7155ba6851ce839bdfa10c18ce2698022cc493ce024b5578c1828e2a94bcdf2552c2f46c392a251ed086691b41959e62a6970821

src/test/fuzz/asmap.cpp

@@ -3,26 +3,47 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <netaddress.h>
-#include <test/fuzz/FuzzedDataProvider.h>
 #include <test/fuzz/fuzz.h>
 
 #include <cstdint>
 #include <vector>
 
+//! asmap code that consumes nothing
+static const std::vector<bool> IPV6_PREFIX_ASMAP = {};
+
+//! asmap code that consumes the 96 prefix bits of ::ffff:0/96 (IPv4-in-IPv6 map)
+static const std::vector<bool> IPV4_PREFIX_ASMAP = {
+    true, true, false, true, true, true, true, true, true, true, false, false, false, false, false, false, false, false, // Match 0x00
+    true, true, false, true, true, true, true, true, true, true, false, false, false, false, false, false, false, false, // Match 0x00
+    true, true, false, true, true, true, true, true, true, true, false, false, false, false, false, false, false, false, // Match 0x00
+    true, true, false, true, true, true, true, true, true, true, false, false, false, false, false, false, false, false, // Match 0x00
+    true, true, false, true, true, true, true, true, true, true, false, false, false, false, false, false, false, false, // Match 0x00
+    true, true, false, true, true, true, true, true, true, true, false, false, false, false, false, false, false, false, // Match 0x00
+    true, true, false, true, true, true, true, true, true, true, false, false, false, false, false, false, false, false, // Match 0x00
+    true, true, false, true, true, true, true, true, true, true, false, false, false, false, false, false, false, false, // Match 0x00
+    true, true, false, true, true, true, true, true, true, true, false, false, false, false, false, false, false, false, // Match 0x00
+    true, true, false, true, true, true, true, true, true, true, false, false, false, false, false, false, false, false, // Match 0x00
+    true, true, false, true, true, true, true, true, true, true, true, true, true, true, true, true, true, true, // Match 0xFF
+    true, true, false, true, true, true, true, true, true, true, true, true, true, true, true, true, true, true // Match 0xFF
+};
+
 void test_one_input(const std::vector<uint8_t>& buffer)
 {
-    FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
-    const Network network = fuzzed_data_provider.PickValueInArray({NET_IPV4, NET_IPV6});
-    if (fuzzed_data_provider.remaining_bytes() < 16) {
-        return;
-    }
-    CNetAddr net_addr;
-    net_addr.SetRaw(network, fuzzed_data_provider.ConsumeBytes<uint8_t>(16).data());
-    std::vector<bool> asmap;
-    for (const char cur_byte : fuzzed_data_provider.ConsumeRemainingBytes<char>()) {
-        for (int bit = 0; bit < 8; ++bit) {
-            asmap.push_back((cur_byte >> bit) & 1);
+    // Encoding: [7 bits: asmap size] [1 bit: ipv6?] [3-130 bytes: asmap] [4 or 16 bytes: addr]
+    if (buffer.size() < 1 + 3 + 4) return;
+    int asmap_size = 3 + (buffer[0] & 127);
+    bool ipv6 = buffer[0] & 128;
+    int addr_size = ipv6 ? 16 : 4;
+    if (buffer.size() < size_t(1 + asmap_size + addr_size)) return;
+    std::vector<bool> asmap = ipv6 ? IPV6_PREFIX_ASMAP : IPV4_PREFIX_ASMAP;
+    asmap.reserve(asmap.size() + 8 * asmap_size);
+    for (int i = 0; i < asmap_size; ++i) {
+        for (int j = 0; j < 8; ++j) {
+            asmap.push_back((buffer[1 + i] >> j) & 1);
         }
     }
+    if (!SanityCheckASMap(asmap)) return;
+    CNetAddr net_addr;
+    net_addr.SetRaw(ipv6 ? NET_IPV6 : NET_IPV4, buffer.data() + 1 + asmap_size);
     (void)net_addr.GetMappedAS(asmap);
 }

src/test/fuzz/asmap_direct.cpp

@@ -0,0 +1,46 @@
+// Copyright (c) 2020 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <util/asmap.h>
+#include <test/fuzz/fuzz.h>
+
+#include <cstdint>
+#include <vector>
+
+#include <assert.h>
+
+void test_one_input(const std::vector<uint8_t>& buffer)
+{
+    // Encoding: [asmap using 1 bit / byte] 0xFF [addr using 1 bit / byte]
+    bool have_sep = false;
+    size_t sep_pos;
+    for (size_t pos = 0; pos < buffer.size(); ++pos) {
+        uint8_t x = buffer[pos];
+        if ((x & 0xFE) == 0) continue;
+        if (x == 0xFF) {
+            if (have_sep) return;
+            have_sep = true;
+            sep_pos = pos;
+        } else {
+            return;
+        }
+    }
+    if (!have_sep) return; // Needs exactly 1 separator
+    if (buffer.size() - sep_pos - 1 > 128) return; // At most 128 bits in IP address
+
+    // Checks on asmap
+    std::vector<bool> asmap(buffer.begin(), buffer.begin() + sep_pos);
+    if (SanityCheckASMap(asmap, buffer.size() - 1 - sep_pos)) {
+        // Verify that for valid asmaps, no prefix (except up to 7 zero padding bits) is valid.
+        std::vector<bool> asmap_prefix = asmap;
+        while (!asmap_prefix.empty() && asmap_prefix.size() + 7 > asmap.size() && asmap_prefix.back() == false) asmap_prefix.pop_back();
+        while (!asmap_prefix.empty()) {
+            asmap_prefix.pop_back();
+            assert(!SanityCheckASMap(asmap_prefix, buffer.size() - 1 - sep_pos));
+        }
+        // No address input should trigger assertions in interpreter
+        std::vector<bool> addr(buffer.begin() + sep_pos + 1, buffer.end());
+        (void)Interpret(asmap, addr);
+    }
+}