Skip to content

Commit b2093f4

Browse files
authored
[dns] check for null character in read label (openthread#13187)
This commit updates `Name::LabelIterator::ReadLabel()` to explicitly check that the read label from the message does not contain any embedded `kNullChar` (`\0`) characters. It uses `StringLength()` to verify that the length of the string matches the expected label length. If a null character is found before the end of the label, `kErrorParse` is returned to prevent potential string truncation issues or misinterpretation of the label name. This broader check replaces a recent fix in `PtrRecord::ReadPtrName()` from openthread#13183 which only verified that the first label was not empty or malformed by checking for a single-character label with a null byte. By enforcing this validation centrally at the `ReadLabel()` level, we now ensure that labels of any length are properly protected against embedded null characters across all DNS record types.
1 parent 15e1c23 commit b2093f4

2 files changed

Lines changed: 27 additions & 4 deletions

File tree

src/core/net/dns_types.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -639,6 +639,9 @@ Error Name::LabelIterator::ReadLabel(char *aLabelBuffer, uint8_t &aLabelLength,
639639
aLabelBuffer[mLabelLength] = kNullChar;
640640
aLabelLength = mLabelLength;
641641

642+
// Check that there is no `kNullChar` (`\0`) in the read label.
643+
VerifyOrExit(StringLength(aLabelBuffer, mLabelLength) == mLabelLength, error = kErrorParse);
644+
642645
if (!aAllowDotCharInLabel)
643646
{
644647
VerifyOrExit(StringFind(aLabelBuffer, kLabelSeparatorChar) == nullptr, error = kErrorParse);
@@ -1592,10 +1595,6 @@ Error PtrRecord::ReadPtrName(const Message &aMessage,
15921595
aOffset = startOffset + sizeof(PtrRecord);
15931596
SuccessOrExit(error = Name::ReadLabel(aMessage, aOffset, aLabelBuffer, aLabelBufferSize));
15941597

1595-
// The first label of a PTR target (the service-instance or host label)
1596-
// must be non-empty.
1597-
VerifyOrExit(aLabelBuffer[0] != kNullChar, error = kErrorParse);
1598-
15991598
if (aNameBuffer != nullptr)
16001599
{
16011600
SuccessOrExit(error = Name::ReadName(aMessage, aOffset, aNameBuffer, aNameBufferSize));

tests/unit/test_dns.cpp

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,14 @@ void TestDnsName(void)
161161
{"_s1._sub._srv._udp.default.service.arpa.", "_s1", "_sub._srv._udp", "default.service.arpa.", true},
162162
};
163163

164+
typedef uint8_t EncodedNameWithNullChar[6];
165+
166+
static const EncodedNameWithNullChar kEncodedNamesWithNullChar[] = {
167+
{4, 0, 'a', 'b', 'c', 0}, // Null char at the start of the label
168+
{4, 'a', 0, 'c', 'd', 0}, // Null char in the middle of the label
169+
{4, 'a', 'b', 'c', 0, 0} // Null char in the end of the label
170+
};
171+
164172
printf("================================================================\n");
165173
printf("TestDnsName()\n");
166174

@@ -626,6 +634,22 @@ void TestDnsName(void)
626634
"012345678901234567890123456789012345678901234567890123456789012") ==
627635
kErrorInvalidArgs);
628636

637+
printf("----------------------------------------------------------------\n");
638+
printf("Name::ReadLabel() when there is null-char \'\\0\' in the label\n");
639+
640+
for (const EncodedNameWithNullChar &encodedName : kEncodedNamesWithNullChar)
641+
{
642+
SuccessOrQuit(message->SetLength(0));
643+
SuccessOrQuit(message->AppendBytes(encodedName, sizeof(encodedName)));
644+
645+
SuccessOrQuit(message->Read(0, buffer, message->GetLength()));
646+
DumpBuffer("EncodedName", buffer, message->GetLength());
647+
648+
offset = 0;
649+
labelLength = sizeof(label);
650+
VerifyOrQuit(Dns::Name::ReadLabel(*message, offset, label, labelLength) == kErrorParse);
651+
}
652+
629653
message->Free();
630654
testFreeInstance(instance);
631655
}

0 commit comments

Comments
 (0)