merge bitcoin#18512: Improve asmap checks and add sanity check

src/Makefile.test.include

@@ -9,6 +9,7 @@ FUZZ_TARGETS = \
   test/fuzz/address_deserialize \
   test/fuzz/addrman_deserialize \
   test/fuzz/asmap \
+  test/fuzz/asmap_direct \
   test/fuzz/banentry_deserialize \
   test/fuzz/base_encode_decode \
   test/fuzz/bech32 \
@@ -292,6 +293,12 @@ test_fuzz_asmap_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
 test_fuzz_asmap_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) $(LDFLAGS_WRAP_EXCEPTIONS)
 test_fuzz_asmap_LDADD = $(FUZZ_SUITE_LD_COMMON)
 
+test_fuzz_asmap_direct_SOURCES = $(FUZZ_SUITE) test/fuzz/asmap_direct.cpp
+test_fuzz_asmap_direct_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES)
+test_fuzz_asmap_direct_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
+test_fuzz_asmap_direct_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) $(LDFLAGS_WRAP_EXCEPTIONS)
+test_fuzz_asmap_direct_LDADD = $(FUZZ_SUITE_LD_COMMON)
+
 test_fuzz_blockheader_deserialize_SOURCES = $(FUZZ_SUITE) test/fuzz/deserialize.cpp
 test_fuzz_blockheader_deserialize_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) -DBLOCKHEADER_DESERIALIZE=1
 test_fuzz_blockheader_deserialize_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)

src/addrman.cpp

@@ -680,5 +680,9 @@ std::vector<bool> CAddrMan::DecodeAsmap(fs::path path)
             bits.push_back((cur_byte >> bit) & 1);
         }
     }
+    if (!SanityCheckASMap(bits)) {
+        LogPrintf("Sanity check of asmap file %s failed\n", path);
+        return {};
+    }
     return bits;
 }

src/netaddress.cpp

@@ -1095,3 +1095,8 @@ bool operator<(const CSubNet& a, const CSubNet& b)
 {
     return (a.network < b.network || (a.network == b.network && memcmp(a.netmask, b.netmask, 16) < 0));
 }
+
+bool SanityCheckASMap(const std::vector<bool>& asmap)
+{
+    return SanityCheckASMap(asmap, 128); // For IP address lookups, the input is 128 bits
+}

src/netaddress.h

@@ -530,4 +530,6 @@ class CService : public CNetAddr
         }
 };
 
+bool SanityCheckASMap(const std::vector<bool>& asmap);
+
 #endif // BITCOIN_NETADDRESS_H

src/test/fuzz/asmap.cpp

@@ -3,26 +3,58 @@
 // 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 <util/asmap.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;
+    const size_t addr_size = ipv6 ? ADDR_IPV6_SIZE : ADDR_IPV4_SIZE;
+    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, 128)) return;
+
+    const uint8_t* addr_data = buffer.data() + 1 + asmap_size;
+    CNetAddr net_addr;
+    if (ipv6) {
+        assert(addr_size == ADDR_IPV6_SIZE);
+        net_addr.SetLegacyIPv6({addr_data, addr_size});
+    } else {
+        assert(addr_size == ADDR_IPV4_SIZE);
+        in_addr ipv4;
+        memcpy(&ipv4, addr_data, addr_size);
+        net_addr.SetIP(CNetAddr{ipv4});
+    }
     (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);
+    }
+}

src/util/asmap.cpp

@@ -2,11 +2,15 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
+#include <assert.h>
+#include <map>
 #include <vector>
 #include <crypto/common.h>
 
 namespace {
 
+constexpr uint32_t INVALID = 0xFFFFFFFF;
+
 uint32_t DecodeBits(std::vector<bool>::const_iterator& bitpos, const std::vector<bool>::const_iterator& endpos, uint8_t minval, const std::vector<uint8_t> &bit_sizes)
 {
     uint32_t val = minval;
@@ -24,7 +28,7 @@ uint32_t DecodeBits(std::vector<bool>::const_iterator& bitpos, const std::vector
             val += (1 << *bit_sizes_it);
         } else {
             for (int b = 0; b < *bit_sizes_it; b++) {
-                if (bitpos == endpos) break;
+                if (bitpos == endpos) return INVALID; // Reached EOF in mantissa
                 bit = *bitpos;
                 bitpos++;
                 val += bit << (*bit_sizes_it - 1 - b);
@@ -32,13 +36,21 @@ uint32_t DecodeBits(std::vector<bool>::const_iterator& bitpos, const std::vector
             return val;
         }
     }
-    return -1;
+    return INVALID; // Reached EOF in exponent
 }
 
-const std::vector<uint8_t> TYPE_BIT_SIZES{0, 0, 1};
-uint32_t DecodeType(std::vector<bool>::const_iterator& bitpos, const std::vector<bool>::const_iterator& endpos)
+enum class Instruction : uint32_t
 {
-    return DecodeBits(bitpos, endpos, 0, TYPE_BIT_SIZES);
+    RETURN = 0,
+    JUMP = 1,
+    MATCH = 2,
+    DEFAULT = 3,
+};
+
+const std::vector<uint8_t> TYPE_BIT_SIZES{0, 0, 1};
+Instruction DecodeType(std::vector<bool>::const_iterator& bitpos, const std::vector<bool>::const_iterator& endpos)
+{
+    return Instruction(DecodeBits(bitpos, endpos, 0, TYPE_BIT_SIZES));
 }
 
 const std::vector<uint8_t> ASN_BIT_SIZES{15, 16, 17, 18, 19, 20, 21, 22, 23, 24};
@@ -69,34 +81,105 @@ uint32_t Interpret(const std::vector<bool> &asmap, const std::vector<bool> &ip)
     const std::vector<bool>::const_iterator endpos = asmap.end();
     uint8_t bits = ip.size();
     uint32_t default_asn = 0;
-    uint32_t opcode, jump, match, matchlen;
+    uint32_t jump, match, matchlen;
+    Instruction opcode;
     while (pos != endpos) {
         opcode = DecodeType(pos, endpos);
-        if (opcode == 0) {
-            return DecodeASN(pos, endpos);
-        } else if (opcode == 1) {
+        if (opcode == Instruction::RETURN) {
+            default_asn = DecodeASN(pos, endpos);
+            if (default_asn == INVALID) break; // ASN straddles EOF
+            return default_asn;
+        } else if (opcode == Instruction::JUMP) {
             jump = DecodeJump(pos, endpos);
-            if (bits == 0) break;
+            if (jump == INVALID) break; // Jump offset straddles EOF
+            if (bits == 0) break; // No input bits left
+            if (jump >= endpos - pos) break; // Jumping past EOF
             if (ip[ip.size() - bits]) {
-                if (jump >= endpos - pos) break;
                 pos += jump;
             }
             bits--;
-        } else if (opcode == 2) {
+        } else if (opcode == Instruction::MATCH) {
             match = DecodeMatch(pos, endpos);
+            if (match == INVALID) break; // Match bits straddle EOF
             matchlen = CountBits(match) - 1;
+            if (bits < matchlen) break; // Not enough input bits
             for (uint32_t bit = 0; bit < matchlen; bit++) {
-                if (bits == 0) break;
                 if ((ip[ip.size() - bits]) != ((match >> (matchlen - 1 - bit)) & 1)) {
                     return default_asn;
                 }
                 bits--;
             }
-        } else if (opcode == 3) {
+        } else if (opcode == Instruction::DEFAULT) {
             default_asn = DecodeASN(pos, endpos);
+            if (default_asn == INVALID) break; // ASN straddles EOF
         } else {
-            break;
+            break; // Instruction straddles EOF
         }
     }
+    assert(false); // Reached EOF without RETURN, or aborted (see any of the breaks above) - should have been caught by SanityCheckASMap below
     return 0; // 0 is not a valid ASN
 }
+
+bool SanityCheckASMap(const std::vector<bool>& asmap, int bits)
+{
+    const std::vector<bool>::const_iterator begin = asmap.begin(), endpos = asmap.end();
+    std::vector<bool>::const_iterator pos = begin;
+    std::vector<std::pair<uint32_t, int>> jumps; // All future positions we may jump to (bit offset in asmap -> bits to consume left)
+    jumps.reserve(bits);
+    Instruction prevopcode = Instruction::JUMP;
+    bool had_incomplete_match = false;
+    while (pos != endpos) {
+        uint32_t offset = pos - begin;
+        if (!jumps.empty() && offset >= jumps.back().first) return false; // There was a jump into the middle of the previous instruction
+        Instruction opcode = DecodeType(pos, endpos);
+        if (opcode == Instruction::RETURN) {
+            if (prevopcode == Instruction::DEFAULT) return false; // There should not be any RETURN immediately after a DEFAULT (could be combined into just RETURN)
+            uint32_t asn = DecodeASN(pos, endpos);
+            if (asn == INVALID) return false; // ASN straddles EOF
+            if (jumps.empty()) {
+                // Nothing to execute anymore
+                if (endpos - pos > 7) return false; // Excessive padding
+                while (pos != endpos) {
+                    if (*pos) return false; // Nonzero padding bit
+                    ++pos;
+                }
+                return true; // Sanely reached EOF
+            } else {
+                // Continue by pretending we jumped to the next instruction
+                offset = pos - begin;
+                if (offset != jumps.back().first) return false; // Unreachable code
+                bits = jumps.back().second; // Restore the number of bits we would have had left after this jump
+                jumps.pop_back();
+                prevopcode = Instruction::JUMP;
+            }
+        } else if (opcode == Instruction::JUMP) {
+            uint32_t jump = DecodeJump(pos, endpos);
+            if (jump == INVALID) return false; // Jump offset straddles EOF
+            if (jump > endpos - pos) return false; // Jump out of range
+            if (bits == 0) return false; // Consuming bits past the end of the input
+            --bits;
+            uint32_t jump_offset = pos - begin + jump;
+            if (!jumps.empty() && jump_offset >= jumps.back().first) return false; // Intersecting jumps
+            jumps.emplace_back(jump_offset, bits);
+            prevopcode = Instruction::JUMP;
+        } else if (opcode == Instruction::MATCH) {
+            uint32_t match = DecodeMatch(pos, endpos);
+            if (match == INVALID) return false; // Match bits straddle EOF
+            int matchlen = CountBits(match) - 1;
+            if (prevopcode != Instruction::MATCH) had_incomplete_match = false;
+            if (matchlen < 8 && had_incomplete_match) return false; // Within a sequence of matches only at most one should be incomplete
+            had_incomplete_match = (matchlen < 8);
+            if (bits < matchlen) return false; // Consuming bits past the end of the input
+            bits -= matchlen;
+            prevopcode = Instruction::MATCH;
+        } else if (opcode == Instruction::DEFAULT) {
+            if (prevopcode == Instruction::DEFAULT) return false; // There should not be two successive DEFAULTs (they could be combined into one)
+            uint32_t asn = DecodeASN(pos, endpos);
+            if (asn == INVALID) return false; // ASN straddles EOF
+            prevopcode = Instruction::DEFAULT;
+        } else {
+            return false; // Instruction straddles EOF
+        }
+    }
+    return false; // Reached EOF without RETURN instruction
+}

src/util/asmap.h

@@ -5,6 +5,11 @@
 #ifndef BITCOIN_UTIL_ASMAP_H
 #define BITCOIN_UTIL_ASMAP_H
 
+#include <stdint.h>
+#include <vector>
+
 uint32_t Interpret(const std::vector<bool> &asmap, const std::vector<bool> &ip);
 
+bool SanityCheckASMap(const std::vector<bool>& asmap, int bits);
+
 #endif // BITCOIN_UTIL_ASMAP_H