From b98563c033d257f443eb7bf59608348c91f0f1ef Mon Sep 17 00:00:00 2001 From: Miod Vallat Date: Mon, 15 Dec 2025 10:53:19 +0100 Subject: [PATCH] Make sure never to try and access LMDBResourceRecords beyond their actual size. Signed-off-by: Miod Vallat --- modules/lmdbbackend/lmdbbackend.cc | 119 +++++++++++++++++++---------- 1 file changed, 78 insertions(+), 41 deletions(-) diff --git a/modules/lmdbbackend/lmdbbackend.cc b/modules/lmdbbackend/lmdbbackend.cc index a4141eb13e..46021e04f4 100644 --- a/modules/lmdbbackend/lmdbbackend.cc +++ b/modules/lmdbbackend/lmdbbackend.cc @@ -1103,6 +1103,9 @@ static inline size_t deserializeRRFromBuffer(const string_view& str, LMDBBackend { const auto* data = str.data(); uint16_t len; + if (str.size() < sizeof(len)) { + return 0; + } memcpy(&len, data, sizeof(len)); if (str.size() < serialize_prefix_size + len + serialize_trailing_size) { return 0; @@ -1123,11 +1126,10 @@ static inline size_t deserializeRRFromBuffer(const string_view& str, LMDBBackend } // Deserialize a single resource record. -static void deserializeFromBuffer(const string_view& buffer, LMDBBackend::LMDBResourceRecord& value) +// Returns true if successful, false if truncated or invalid data found. +static bool deserializeFromBuffer(const string_view& buffer, LMDBBackend::LMDBResourceRecord& value) { - if (buffer.size() >= serialize_minimum_size) { - deserializeRRFromBuffer(buffer, value); - } + return deserializeRRFromBuffer(buffer, value) != 0; } // Deserialize a list of resource records. @@ -1171,28 +1173,49 @@ static std::shared_ptr deserializeContentZR(uint16_t qtype, co // } static bool peekAtHasOrderName(const string_view& buffer) { + if (buffer.length() < sizeof(uint16_t)) { + return false; + } uint16_t len{0}; memcpy(&len, buffer.data(), sizeof(uint16_t)); - bool hasOrderName = buffer[serialize_prefix_size + len + serialize_offset_ordername] != 0; + auto offset = serialize_prefix_size + len + serialize_offset_ordername; + if (buffer.length() < offset + 1) { + return false; + } + bool hasOrderName = buffer[offset] != 0; return hasOrderName; } // Similar to the above, but for the auth field. static bool peekAtAuth(const string_view& buffer) { + if (buffer.length() < sizeof(uint16_t)) { + return false; + } uint16_t len{0}; memcpy(&len, buffer.data(), sizeof(uint16_t)); - bool auth = buffer[serialize_prefix_size + len + serialize_offset_auth] != 0; + auto offset = serialize_prefix_size + len + serialize_offset_auth; + if (buffer.length() < offset + 1) { + return false; + } + bool auth = buffer[offset] != 0; return auth; } // Similar to the above, but for the ttl. static uint32_t peekAtTtl(const string_view& buffer) { + if (buffer.length() < sizeof(uint16_t)) { + return 0; + } uint16_t len{0}; memcpy(&len, buffer.data(), sizeof(uint16_t)); + auto offset = serialize_prefix_size + len + serialize_offset_ttl; + if (buffer.length() < offset + sizeof(uint32_t)) { + return 0; + } uint32_t ttl{0}; - memcpy(&ttl, buffer.data() + serialize_prefix_size + len + serialize_offset_ttl, sizeof(uint32_t)); + memcpy(&ttl, buffer.data() + offset, sizeof(uint32_t)); return ttl; } @@ -1355,9 +1378,10 @@ void LMDBBackend::deleteNSEC3RecordPair(const std::shared_ptrtxn->get(txn->db->dbi, key, val) == 0) { LMDBResourceRecord lrr; - deserializeFromBuffer(val.get(), lrr); - DNSName ordername(lrr.content.c_str(), lrr.content.size(), 0, false); - txn->txn->del(txn->db->dbi, co(domain_id, ordername, QType::NSEC3)); + if (deserializeFromBuffer(val.get(), lrr)) { + DNSName ordername(lrr.content.c_str(), lrr.content.size(), 0, false); + txn->txn->del(txn->db->dbi, co(domain_id, ordername, QType::NSEC3)); + } txn->txn->del(txn->db->dbi, key); } } @@ -1381,12 +1405,13 @@ void LMDBBackend::writeNSEC3RecordPair(const std::shared_ptrtxn->get(txn->db->dbi, co(domain_id, qname, QType::NSEC3), val) == 0) { LMDBResourceRecord lrr; - deserializeFromBuffer(val.get(), lrr); - DNSName prevordername(lrr.content.c_str(), lrr.content.size(), 0, false); - if (prevordername == ordername) { - return; // nothing to do! (assuming the other record also exists) + if (deserializeFromBuffer(val.get(), lrr)) { + DNSName prevordername(lrr.content.c_str(), lrr.content.size(), 0, false); + if (prevordername == ordername) { + return; // nothing to do! (assuming the other record also exists) + } + txn->txn->del(txn->db->dbi, co(domain_id, prevordername, QType::NSEC3)); } - txn->txn->del(txn->db->dbi, co(domain_id, prevordername, QType::NSEC3)); } LMDBResourceRecord lrr; @@ -2038,8 +2063,10 @@ bool LMDBBackend::getInternal(DNSName& basename, std::string_view& key) } deserializeMultipleFromBuffer(d_lookupstate.val.get(), d_lookupstate.rrset); - d_lookupstate.rrsettime = static_cast(LMDBLS::LSgetTimestamp(d_lookupstate.val.getNoStripHeader()) / (1000UL * 1000UL * 1000UL)); - d_lookupstate.rrsetpos = 0; + if (!d_lookupstate.rrset.empty()) { + d_lookupstate.rrsettime = static_cast(LMDBLS::LSgetTimestamp(d_lookupstate.val.getNoStripHeader()) / (1000UL * 1000UL * 1000UL)); + d_lookupstate.rrsetpos = 0; + } } else { key = d_lookupstate.key.getNoStripHeader(); @@ -2126,15 +2153,16 @@ bool LMDBBackend::getSerial(DomainInfo& di) MDBOutVal val; if (!txn->txn->get(txn->db->dbi, co(di.id, g_rootdnsname, QType::SOA), val)) { LMDBResourceRecord lrr; - deserializeFromBuffer(val.get(), lrr); - if (lrr.content.size() >= sizeof(soatimes)) { - soatimes st; - // A SOA has five 32 bit fields, the first of which is the serial; - // there are two variable length names before the serial, so we calculate from the back. - memcpy(&st.serial, &lrr.content[lrr.content.size() - sizeof(soatimes)], sizeof(st.serial)); - di.serial = ntohl(st.serial); + if (deserializeFromBuffer(val.get(), lrr)) { + if (lrr.content.size() >= sizeof(soatimes)) { + soatimes st; + // A SOA has five 32 bit fields, the first of which is the serial; + // there are two variable length names before the serial, so we calculate from the back. + memcpy(&st.serial, &lrr.content[lrr.content.size() - sizeof(soatimes)], sizeof(st.serial)); + di.serial = ntohl(st.serial); + } + return !lrr.disabled; } - return !lrr.disabled; } return false; } @@ -2306,15 +2334,17 @@ void LMDBBackend::getUnfreshSecondaryInfos(vector* domains) MDBOutVal val; if (!txn2->txn->get(txn2->db->dbi, co(di.id, g_rootdnsname, QType::SOA), val)) { LMDBResourceRecord lrr; - deserializeFromBuffer(val.get(), lrr); - soatimes st; - // There are two variable length names before the SOA numbers, so we calculate from the back. - memcpy(&st, &lrr.content[lrr.content.size() - sizeof(soatimes)], sizeof(soatimes)); - if ((time_t)(di.last_check + ntohl(st.refresh)) > now) { // still fresh - return false; + if (deserializeFromBuffer(val.get(), lrr)) { + if (lrr.content.size() >= sizeof(soatimes)) { + soatimes st; + // There are two variable length names before the SOA numbers, so we calculate from the back. + memcpy(&st, &lrr.content[lrr.content.size() - sizeof(soatimes)], sizeof(soatimes)); + if ((time_t)(di.last_check + ntohl(st.refresh)) > now) { // still fresh + return false; + } + } } } - return true; }); } @@ -2702,8 +2732,9 @@ bool LMDBBackend::getBeforeAndAfterNamesAbsolute(domainid_t id, const DNSName& q before = co.getQName(key.getNoStripHeader()); { LMDBResourceRecord lrr; - deserializeFromBuffer(val.get(), lrr); - unhashed = DNSName(lrr.content.c_str(), lrr.content.size(), 0, false) + info.zone.operator const DNSName&(); + if (deserializeFromBuffer(val.get(), lrr)) { + unhashed = DNSName(lrr.content.c_str(), lrr.content.size(), 0, false) + info.zone.operator const DNSName&(); + } } // now to find after .. at the beginning of the zone return getAfterForwardFromStart(cursor, key, val, id, after); @@ -2761,8 +2792,9 @@ bool LMDBBackend::getBeforeAndAfterNamesAbsolute(domainid_t id, const DNSName& q before = co.getQName(key.getNoStripHeader()); { LMDBResourceRecord lrr; - deserializeFromBuffer(val.get(), lrr); - unhashed = DNSName(lrr.content.c_str(), lrr.content.size(), 0, false) + info.zone.operator const DNSName&(); + if (deserializeFromBuffer(val.get(), lrr)) { + unhashed = DNSName(lrr.content.c_str(), lrr.content.size(), 0, false) + info.zone.operator const DNSName&(); + } } // cout <<"Should still find 'after'!"<()); { LMDBResourceRecord lrr; - deserializeFromBuffer(val.get(), lrr); - unhashed = DNSName(lrr.content.c_str(), lrr.content.size(), 0, false) + info.zone.operator const DNSName&(); + if (deserializeFromBuffer(val.get(), lrr)) { + unhashed = DNSName(lrr.content.c_str(), lrr.content.size(), 0, false) + info.zone.operator const DNSName&(); + } } // cout<<"Went backwards, found "<& argv) MDBOutVal val{}; if (d_rotxn->txn->get(d_rotxn->db->dbi, order(info.id, basename, QType::NSEC3), val) == 0) { LMDBResourceRecord nsec3rr; - deserializeFromBuffer(val.get(), nsec3rr); - DNSName ordername(nsec3rr.content.c_str(), nsec3rr.content.size(), 0, false); - ret << ordername; + if (deserializeFromBuffer(val.get(), nsec3rr)) { + DNSName ordername(nsec3rr.content.c_str(), nsec3rr.content.size(), 0, false); + ret << ordername; + } + else { + ret << "TRUNCATED"; + } } ret << "'\t"; ret << static_cast(lrr.auth); -- 2.47.3