Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions src/evo/netinfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ static std::once_flag g_main_params_flag;

/** Maximum length of a label in a domain per RFC 1035 */
static constexpr uint8_t DOMAIN_LABEL_MAX_LEN{63};
/** Maximum possible length of a ASCII FQDN */
static constexpr uint8_t DOMAIN_MAX_LEN{253};
/** Minimum length of a FQDN */
static constexpr uint8_t DOMAIN_MIN_LEN{3};

Expand Down
20 changes: 14 additions & 6 deletions src/evo/netinfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ class UniValue;
/** Maximum entries that can be stored in an ExtNetInfo per purpose code */
static constexpr uint8_t MAX_ENTRIES_EXTNETINFO{4};

/** Maximum possible length of an ASCII FQDN (RFC 1035). Applied at the
* serialization layer so oversized CompactSize claims are rejected before
* allocate-and-zero; ValidateDomain enforces the same ceiling after decode. */
static constexpr size_t DOMAIN_MAX_LEN{253};

enum class NetInfoStatus : uint8_t {
// Managing entries
BadInput,
Expand Down Expand Up @@ -170,7 +175,7 @@ class DomainPort

SERIALIZE_METHODS(DomainPort, obj)
{
READWRITE(obj.m_addr);
READWRITE(LIMITED_STRING(obj.m_addr, DOMAIN_MAX_LEN));
READWRITE(Using<BigEndianFormatter<2>>(obj.m_port));
}

Expand Down Expand Up @@ -246,11 +251,14 @@ class NetInfoEntry
if (!service.IsValid()) { Clear(); } // Invalid CService, mark as invalid
} catch (const std::ios_base::failure&) { Clear(); } // Deser failed, mark as invalid
} else if (m_type == NetInfoType::Domain) {
try {
auto& domain{m_data.emplace<DomainPort>()};
s >> domain;
if (!domain.IsValid()) { Clear(); } // Invalid DomainPort, mark as invalid
} catch (const std::ios_base::failure&) { Clear(); } // Deser failed, mark as invalid
// Do not swallow deserialization failures: a short/oversized domain
// body would otherwise leave the stream position unmoved (or only
// advanced by CompactSize) while the enclosing vector loop continues,
// amplifying a few attacker bytes into repeated allocations.
// Propagate ios_base::failure so the whole ProTx payload is rejected.
auto& domain{m_data.emplace<DomainPort>()};
s >> domain;
if (!domain.IsValid()) { Clear(); } // Invalid DomainPort, mark as invalid
} else { Clear(); } // Invalid type code, mark as invalid
}

Expand Down
86 changes: 86 additions & 0 deletions src/test/evo_netinfo_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,92 @@ BOOST_AUTO_TEST_CASE(domainport_rules)
}
}

// DomainPort used a bare std::string unserialize path (bounded only by
// MAX_SIZE == 32 MiB) and NetInfoEntry::Unserialize swallowed ios_base::failure.
// That combination allowed a tiny ProTx ExtNetInfo payload to force repeated
// allocate-and-zero of multi-megabyte strings. These tests pin the serialization
// layer to reject oversized domain claims without performing the large allocation.
static CDataStream MakeOversizedDomainPortStream(const size_t claimed_len)
{
CDataStream ds(SER_DISK, CLIENT_VERSION);
WriteCompactSize(ds, claimed_len);
// Provide a full body so pre-fix code can complete the string read; the
// bound must reject based on the CompactSize alone, not on a short read.
const std::string body(claimed_len, 'a');
if (!body.empty()) {
ds.write(MakeByteSpan(body));
}
ser_writedata16be(ds, 443);
return ds;
}

BOOST_AUTO_TEST_CASE(domainport_deser_rejects_oversized_string)
{
// DOMAIN_MAX_LEN is 253; a fully-formed 1000-byte domain must be rejected at
// deserialization time (LIMITED_STRING), not accepted then failed later in
// ValidateDomain.
constexpr size_t kOversized{1000};
CDataStream ds{MakeOversizedDomainPortStream(kOversized)};

DomainPort domain;
BOOST_CHECK_THROW(ds >> domain, std::ios_base::failure);
}

BOOST_AUTO_TEST_CASE(netinfoentry_domain_deser_rejects_oversized_string)
{
// NetInfoEntry must not swallow the length-limit failure: rethrow so the
// enclosing ProTx payload deserialization aborts and the peer is penalised.
constexpr size_t kOversized{1000};
CDataStream ds(SER_DISK, CLIENT_VERSION);
ds << uint8_t{NetInfoEntry::NetInfoType::Domain};
{
CDataStream body{MakeOversizedDomainPortStream(kOversized)};
ds.write(MakeByteSpan(body));
}

NetInfoEntry entry;
BOOST_CHECK_THROW(ds >> entry, std::ios_base::failure);
}

BOOST_AUTO_TEST_CASE(domainport_deser_rejects_huge_claimed_length)
{
// CompactSize claims 1 MiB with no body. Pre-fix this would allocate 1 MiB
// then throw on short-read; post-fix LIMITED_STRING rejects before resize.
// Either way the operation must throw; the important property is that a
// declared length above DOMAIN_MAX_LEN never produces a successful DomainPort.
CDataStream ds(SER_DISK, CLIENT_VERSION);
WriteCompactSize(ds, size_t{1} << 20);

DomainPort domain;
BOOST_CHECK_THROW(ds >> domain, std::ios_base::failure);
}

BOOST_AUTO_TEST_CASE(extnetinfo_domain_amplification_rejected)
{
// Simulate the attack shape: ExtNetInfo vector of Domain entries each
// claiming a large string length from a small stream. Pre-fix,
// NetInfoEntry swallows the short-read and the vector loop continues
// (stream position does not advance on the failed body read), producing
// repeated large allocations. Post-fix the first oversized claim throws
// and aborts the whole ExtNetInfo unserialize.
constexpr size_t kClaimed{1 << 16}; // 64 KiB — large enough to prove, small enough to not OOM pre-fix
constexpr size_t kEntries{8};

CDataStream ds(SER_DISK, CLIENT_VERSION);
ds << uint8_t{1}; // ExtNetInfo version
WriteCompactSize(ds, 1); // one purpose
ds << NetInfoPurpose::PLATFORM_HTTPS;
WriteCompactSize(ds, kEntries);
for (size_t i = 0; i < kEntries; ++i) {
ds << uint8_t{NetInfoEntry::NetInfoType::Domain};
// Claim kClaimed bytes but supply none — CompactSize only.
WriteCompactSize(ds, kClaimed);
}

ExtNetInfo netInfo;
BOOST_CHECK_THROW(ds >> netInfo, std::ios_base::failure);
}

BOOST_FIXTURE_TEST_CASE(extnetinfo_validate_deser, RegTestingSetup)
{
// The in-memory builder (AddEntry/ProcessCandidate) refuses duplicates, domains
Expand Down
Loading