diff --git a/groups/ntc/ntcdns/ntcdns_protocol.cpp b/groups/ntc/ntcdns/ntcdns_protocol.cpp index a883e7a2..55624b2c 100644 --- a/groups/ntc/ntcdns/ntcdns_protocol.cpp +++ b/groups/ntc/ntcdns/ntcdns_protocol.cpp @@ -673,25 +673,36 @@ ntsa::Error MemoryDecoder::decodeLabel(bsl::string* value, } ntsa::Error MemoryDecoder::decodeCharacterString(bsl::string* value) +{ + return this->decodeCharacterString(value, d_end - d_current); +} + +ntsa::Error MemoryDecoder::decodeCharacterString(bsl::string* value, + bsl::size_t limit) { ntsa::Error error; + if (limit > static_cast(d_end - d_current)) { + limit = d_end - d_current; + } + bsl::uint8_t length = 0; - error = Validation::checkUnderflow(d_end - d_current, sizeof length); + error = Validation::checkUnderflow(limit, sizeof length); + if (error) { + return error; + } + + length = *d_current; + + error = Validation::checkUnderflow(limit, sizeof length + length); if (error) { return error; } - length = *d_current; d_current += sizeof length; if (length > 0) { - error = Validation::checkUnderflow(d_end - d_current, length); - if (error) { - return error; - } - value->append(reinterpret_cast(d_current), static_cast(length)); @@ -1550,6 +1561,12 @@ ntsa::Error ResourceRecord::decode(MemoryDecoder* decoder) return error; } + error = Validation::checkUnderflow(decoder->end() - decoder->current(), + rdataLength); + if (error) { + return error; + } + if (rdataLength > 0) { bsl::size_t p0 = decoder->position(); @@ -1627,6 +1644,13 @@ ntsa::Error ResourceRecord::decode(MemoryDecoder* decoder) ntcdns::ResourceRecordDataWks& rdata = d_rdata.makeWellKnownService(); + error = Validation::checkUnderflow( + rdataLength, + sizeof(bsl::uint32_t) + sizeof(bsl::uint8_t)); + if (error) { + return error; + } + error = decoder->decodeRaw(&rdata.address(), sizeof(bsl::uint32_t)); if (error) { @@ -1672,12 +1696,14 @@ ntsa::Error ResourceRecord::decode(MemoryDecoder* decoder) else if (d_type == ntcdns::Type::e_HINFO) { ntcdns::ResourceRecordDataHinfo& rdata = d_rdata.makeHostInfo(); - error = decoder->decodeCharacterString(&rdata.cpu()); + error = decoder->decodeCharacterString(&rdata.cpu(), rdataLength); if (error) { return error; } - error = decoder->decodeCharacterString(&rdata.os()); + error = decoder->decodeCharacterString( + &rdata.os(), + rdataLength - (decoder->position() - p0)); if (error) { return error; } @@ -1704,7 +1730,8 @@ ntsa::Error ResourceRecord::decode(MemoryDecoder* decoder) bsl::size_t s0 = decoder->position(); rdata.text().resize(rdata.text().size() + 1); - error = decoder->decodeCharacterString(&rdata.text().back()); + error = decoder->decodeCharacterString(&rdata.text().back(), + numBytesRemaining); if (error) { return error; } diff --git a/groups/ntc/ntcdns/ntcdns_protocol.h b/groups/ntc/ntcdns/ntcdns_protocol.h index 7e658416..36ab808f 100644 --- a/groups/ntc/ntcdns/ntcdns_protocol.h +++ b/groups/ntc/ntcdns/ntcdns_protocol.h @@ -230,6 +230,11 @@ class MemoryDecoder /// Decode the specified character string 'value'. Return the error. ntsa::Error decodeCharacterString(bsl::string* value); + /// Decode the specified character string 'value', failing without + /// advancing if the encoded character string, including its length + /// octet, is greater than the specified 'limit'. Return the error. + ntsa::Error decodeCharacterString(bsl::string* value, bsl::size_t limit); + /// Decode the specified raw resource record data 'value'. Return the /// error. ntsa::Error decodeRdata(bdlbb::Blob* value); diff --git a/groups/ntc/ntcdns/ntcdns_protocol.t.cpp b/groups/ntc/ntcdns/ntcdns_protocol.t.cpp index 0e6fcd08..a7372de1 100644 --- a/groups/ntc/ntcdns/ntcdns_protocol.t.cpp +++ b/groups/ntc/ntcdns/ntcdns_protocol.t.cpp @@ -67,8 +67,93 @@ class ProtocolTest // TODO static void verifyWks(); + + // Verify character strings are decoded within a limit. + static void verifyCharacterStringLimit(); + + // Verify TXT records. + static void verifyTxt(); + + // Verify malformed TXT records are rejected. + static void verifyTxtMalformed(); + + // Verify HINFO records. + static void verifyHinfo(); + + // Verify malformed HINFO records are rejected. + static void verifyHinfoMalformed(); + + // Verify malformed WKS records are rejected. + static void verifyWksMalformed(); + + private: + // Load into the specified 'result' a response to a question for the + // specified 'type' of "example.com" having the specified 'ancount' + // answers, the first of which has the specified 'type', the specified + // 'rdlength', and is followed by the specified 'payload' having the + // specified 'payloadSize'. Note that 'payloadSize' may differ from + // 'rdlength' and 'payload' may contain subsequent answers. + static void buildResponse(bsl::vector* result, + bsl::uint16_t type, + bsl::uint16_t ancount, + bsl::uint16_t rdlength, + const bsl::uint8_t* payload, + bsl::size_t payloadSize); + + // Verify the specified 'message' survives an encode/decode round trip. + static void verifyRoundTrip(const ntcdns::Message& message); }; +void ProtocolTest::buildResponse(bsl::vector* result, + bsl::uint16_t type, + bsl::uint16_t ancount, + bsl::uint16_t rdlength, + const bsl::uint8_t* payload, + bsl::size_t payloadSize) +{ + const bsl::uint8_t typeHi = static_cast(type >> 8); + const bsl::uint8_t typeLo = static_cast(type & 0xFF); + + // clang-format off + const bsl::uint8_t PREFIX[] = { + 0x12, 0x34, 0x84, 0x00, 0x00, 0x01, + static_cast(ancount >> 8), + static_cast(ancount & 0xFF), + 0x00, 0x00, 0x00, 0x00, + 0x07, 0x65, 0x78, 0x61, 0x6d, 0x70, 0x6c, 0x65, + 0x03, 0x63, 0x6f, 0x6d, 0x00, + typeHi, typeLo, 0x00, 0x01, + 0xc0, 0x0c, typeHi, typeLo, 0x00, 0x01, + 0x00, 0x00, 0x0e, 0x10, + static_cast(rdlength >> 8), + static_cast(rdlength & 0xFF) + }; + // clang-format on + + result->assign(PREFIX, PREFIX + sizeof PREFIX); + result->insert(result->end(), payload, payload + payloadSize); +} + +void ProtocolTest::verifyRoundTrip(const ntcdns::Message& message) +{ + ntsa::Error error; + + bsl::vector buffer(1024 * 64); + + ntcdns::MemoryEncoder encoder(&buffer[0], buffer.size()); + + error = message.encode(&encoder); + NTSCFG_TEST_EQ(error, ntsa::Error(ntsa::Error::e_OK)); + + ntcdns::MemoryDecoder decoder(&buffer[0], encoder.position()); + + ntcdns::Message other(NTSCFG_TEST_ALLOCATOR); + error = other.decode(&decoder); + NTSCFG_TEST_EQ(error, ntsa::Error(ntsa::Error::e_OK)); + + NTSCFG_TEST_EQ(message, other); +} + NTSCFG_TEST_FUNCTION(ntcdns::ProtocolTest::verifyCase1) { // Concern: Real request data. @@ -611,5 +696,282 @@ NTSCFG_TEST_FUNCTION(ntcdns::ProtocolTest::verifyWks) } } +NTSCFG_TEST_FUNCTION(ntcdns::ProtocolTest::verifyCharacterStringLimit) +{ + // Concern: A character string whose declared length exceeds the limit + // is rejected without advancing the decoder. + + ntsa::Error error; + + const bsl::uint8_t DATA[] = {0x05, 0x68, 0x65, 0x6c, 0x6c, 0x6f}; + + { + ntcdns::MemoryDecoder decoder(DATA, sizeof DATA); + + bsl::string value(NTSCFG_TEST_ALLOCATOR); + error = decoder.decodeCharacterString(&value, 3); + NTSCFG_TEST_EQ(error, ntsa::Error(ntsa::Error::e_INVALID)); + NTSCFG_TEST_EQ(decoder.position(), 0); + NTSCFG_TEST_TRUE(value.empty()); + + error = decoder.decodeCharacterString(&value, 0); + NTSCFG_TEST_EQ(error, ntsa::Error(ntsa::Error::e_INVALID)); + NTSCFG_TEST_EQ(decoder.position(), 0); + + error = decoder.decodeCharacterString(&value, sizeof DATA); + NTSCFG_TEST_EQ(error, ntsa::Error(ntsa::Error::e_OK)); + NTSCFG_TEST_EQ(decoder.position(), sizeof DATA); + NTSCFG_TEST_EQ(value, "hello"); + } + + { + // A limit greater than the buffer is clamped to the buffer. + + ntcdns::MemoryDecoder decoder(DATA, 2); + + bsl::string value(NTSCFG_TEST_ALLOCATOR); + error = decoder.decodeCharacterString(&value, 100); + NTSCFG_TEST_EQ(error, ntsa::Error(ntsa::Error::e_INVALID)); + NTSCFG_TEST_EQ(decoder.position(), 0); + } +} + +NTSCFG_TEST_FUNCTION(ntcdns::ProtocolTest::verifyTxt) +{ + // Concern: TXT records containing multiple character strings, including + // an empty character string. + + ntsa::Error error; + + // clang-format off + const bsl::uint8_t RDATA[] = { + 0x05, 0x68, 0x65, 0x6c, 0x6c, 0x6f, + 0x00, + 0x05, 0x77, 0x6f, 0x72, 0x6c, 0x64 + }; + // clang-format on + + bsl::vector response(NTSCFG_TEST_ALLOCATOR); + ProtocolTest::buildResponse(&response, + ntcdns::Type::e_TXT, + 1, + sizeof RDATA, + RDATA, + sizeof RDATA); + + ntcdns::Message message(NTSCFG_TEST_ALLOCATOR); + + { + ntcdns::MemoryDecoder decoder(&response[0], response.size()); + error = message.decode(&decoder); + NTSCFG_TEST_EQ(error, ntsa::Error(ntsa::Error::e_OK)); + } + + NTSCFG_TEST_EQ(message.ancount(), 1); + + const ntcdns::ResourceRecord& answer = message.an(0); + + NTSCFG_TEST_EQ(answer.type(), ntcdns::Type::e_TXT); + NTSCFG_TEST_TRUE(answer.rdata().isTextValue()); + + const ntcdns::ResourceRecordDataTxt& txt = answer.rdata().text(); + + NTSCFG_TEST_EQ(txt.text().size(), 3); + NTSCFG_TEST_EQ(txt.text()[0], "hello"); + NTSCFG_TEST_EQ(txt.text()[1], ""); + NTSCFG_TEST_EQ(txt.text()[2], "world"); + + ProtocolTest::verifyRoundTrip(message); +} + +NTSCFG_TEST_FUNCTION(ntcdns::ProtocolTest::verifyTxtMalformed) +{ + // Concern: TXT records whose character strings are inconsistent with + // the declared RDATA length are rejected. + + ntsa::Error error; + + { + // A character string that extends past the RDATA into a subsequent, + // well-formed answer. + + // clang-format off + const bsl::uint8_t PAYLOAD[] = { + 0x05, 0x61, 0x62, 0x63, + 0xc0, 0x0c, 0x00, 0x10, 0x00, 0x01, + 0x00, 0x00, 0x0e, 0x10, 0x00, 0x02, + 0x01, 0x78 + }; + // clang-format on + + bsl::vector response(NTSCFG_TEST_ALLOCATOR); + ProtocolTest::buildResponse(&response, + ntcdns::Type::e_TXT, + 2, + 4, + PAYLOAD, + sizeof PAYLOAD); + + { + ntcdns::MemoryDecoder decoder(&response[0], response.size()); + ntcdns::Message message(NTSCFG_TEST_ALLOCATOR); + error = message.decode(&decoder); + NTSCFG_TEST_EQ(error, ntsa::Error(ntsa::Error::e_INVALID)); + } + + { + // The decoder must not advance past the RDATA of the first + // answer, which begins immediately after the response prefix. + + const bsl::size_t answerOffset = 29; + const bsl::size_t rdataOffset = response.size() - sizeof PAYLOAD; + + ntcdns::MemoryDecoder decoder(&response[0], response.size()); + error = decoder.seek(answerOffset); + NTSCFG_TEST_EQ(error, ntsa::Error(ntsa::Error::e_OK)); + + ntcdns::ResourceRecord answer(NTSCFG_TEST_ALLOCATOR); + error = answer.decode(&decoder); + NTSCFG_TEST_EQ(error, ntsa::Error(ntsa::Error::e_INVALID)); + NTSCFG_TEST_EQ(decoder.position(), rdataOffset); + } + } + + { + // The declared RDATA length extends past the end of the message. + + const bsl::uint8_t PAYLOAD[] = {0x05, 0x68, 0x65, 0x6c, 0x6c, 0x6f}; + + bsl::vector response(NTSCFG_TEST_ALLOCATOR); + ProtocolTest::buildResponse(&response, + ntcdns::Type::e_TXT, + 1, + 32, + PAYLOAD, + sizeof PAYLOAD); + + ntcdns::MemoryDecoder decoder(&response[0], response.size()); + ntcdns::Message message(NTSCFG_TEST_ALLOCATOR); + error = message.decode(&decoder); + NTSCFG_TEST_EQ(error, ntsa::Error(ntsa::Error::e_INVALID)); + } + + { + // The declared RDATA length ends within a character string. + + const bsl::uint8_t PAYLOAD[] = {0x05, 0x68, 0x65, 0x6c, 0x6c, 0x6f}; + + bsl::vector response(NTSCFG_TEST_ALLOCATOR); + ProtocolTest::buildResponse(&response, + ntcdns::Type::e_TXT, + 1, + 2, + PAYLOAD, + sizeof PAYLOAD); + + ntcdns::MemoryDecoder decoder(&response[0], response.size()); + ntcdns::Message message(NTSCFG_TEST_ALLOCATOR); + error = message.decode(&decoder); + NTSCFG_TEST_EQ(error, ntsa::Error(ntsa::Error::e_INVALID)); + } +} + +NTSCFG_TEST_FUNCTION(ntcdns::ProtocolTest::verifyHinfo) +{ + // Concern: HINFO records. + + ntsa::Error error; + + // clang-format off + const bsl::uint8_t RDATA[] = { + 0x03, 0x78, 0x38, 0x36, + 0x05, 0x4c, 0x69, 0x6e, 0x75, 0x78 + }; + // clang-format on + + bsl::vector response(NTSCFG_TEST_ALLOCATOR); + ProtocolTest::buildResponse(&response, + ntcdns::Type::e_HINFO, + 1, + sizeof RDATA, + RDATA, + sizeof RDATA); + + ntcdns::Message message(NTSCFG_TEST_ALLOCATOR); + + { + ntcdns::MemoryDecoder decoder(&response[0], response.size()); + error = message.decode(&decoder); + NTSCFG_TEST_EQ(error, ntsa::Error(ntsa::Error::e_OK)); + } + + const ntcdns::ResourceRecord& answer = message.an(0); + + NTSCFG_TEST_EQ(answer.type(), ntcdns::Type::e_HINFO); + NTSCFG_TEST_TRUE(answer.rdata().isHostInfoValue()); + NTSCFG_TEST_EQ(answer.rdata().hostInfo().cpu(), "x86"); + NTSCFG_TEST_EQ(answer.rdata().hostInfo().os(), "Linux"); + + ProtocolTest::verifyRoundTrip(message); +} + +NTSCFG_TEST_FUNCTION(ntcdns::ProtocolTest::verifyHinfoMalformed) +{ + // Concern: HINFO records whose character strings are inconsistent with + // the declared RDATA length are rejected. + + ntsa::Error error; + + // clang-format off + const bsl::uint8_t PAYLOAD[] = { + 0x03, 0x78, 0x38, 0x36, + 0x05, 0x4c, 0x69, 0x6e, 0x75, 0x78 + }; + // clang-format on + + // The declared RDATA length ends within the CPU, after the CPU, and + // within the OS, respectively. + + const bsl::uint16_t RDLENGTH[] = {2, 4, 7}; + + for (bsl::size_t i = 0; i < sizeof RDLENGTH / sizeof RDLENGTH[0]; ++i) { + bsl::vector response(NTSCFG_TEST_ALLOCATOR); + ProtocolTest::buildResponse(&response, + ntcdns::Type::e_HINFO, + 1, + RDLENGTH[i], + PAYLOAD, + sizeof PAYLOAD); + + ntcdns::MemoryDecoder decoder(&response[0], response.size()); + ntcdns::Message message(NTSCFG_TEST_ALLOCATOR); + error = message.decode(&decoder); + NTSCFG_TEST_EQ(error, ntsa::Error(ntsa::Error::e_INVALID)); + } +} + +NTSCFG_TEST_FUNCTION(ntcdns::ProtocolTest::verifyWksMalformed) +{ + // Concern: WKS records whose declared RDATA length is less than the size + // of the address and protocol are rejected. + + ntsa::Error error; + + const bsl::uint8_t PAYLOAD[] = {0xc0, 0xa8, 0x01, 0x01, 0x06, 0x00, 0x00}; + + bsl::vector response(NTSCFG_TEST_ALLOCATOR); + ProtocolTest::buildResponse(&response, + ntcdns::Type::e_WKS, + 1, + 3, + PAYLOAD, + sizeof PAYLOAD); + + ntcdns::MemoryDecoder decoder(&response[0], response.size()); + ntcdns::Message message(NTSCFG_TEST_ALLOCATOR); + error = message.decode(&decoder); + NTSCFG_TEST_EQ(error, ntsa::Error(ntsa::Error::e_INVALID)); +} + } // close namespace ntcdns } // close namespace BloombergLP