Skip to content

Commit

Permalink
[OpenThread] Harden DNS record parsing
Browse files Browse the repository at this point in the history
OpenThread applications would crash upon receiving an empty
DNS TXT record. The reason was that the code for copying OT
DNS service info object into Matter DnssdService object
would not initialize the TXT entry count in the latter
object in such a case.

In the reported case, the Matter stack was presented an
empty TXT record because OpenThread's DNS client received
a TXT record with TTL 0 and it discarded its contents.
Nevertheless, the issue could be reproduced by publishing
Matter service without TXT entries and kicking off DNS query.

1. Initialize the TXT entry and subtype count properly in all
   scenarios.
2. Do not even process the service info object if an error was
   returned by OpenThread before.
3. Extract some boilerplate to a separate function to improve
   readability.

Signed-off-by: Damian Krolik <[email protected]>
  • Loading branch information
Damian-Nordic committed Jan 3, 2024
1 parent 827f6ca commit cfb6b1d
Showing 1 changed file with 35 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,29 @@ void initNetworkCommissioningThreadDriver(void)
#endif
}

#if CHIP_DEVICE_CONFIG_ENABLE_THREAD_DNS_CLIENT
CHIP_ERROR ReadDomainNameComponent(const char *& in, char * out, size_t outSize)
{
const char * dotPos = strchr(in, '.');
VerifyOrReturnError(dotPos != nullptr, CHIP_ERROR_INVALID_ARGUMENT);

const size_t componentSize = static_cast<size_t>(dotPos - in);
VerifyOrReturnError(componentSize < outSize, CHIP_ERROR_INVALID_ARGUMENT);

memcpy(out, in, componentSize);
out[componentSize] = '\0';
in += componentSize + 1;

return CHIP_NO_ERROR;
}

template <size_t N>
CHIP_ERROR ReadDomainNameComponent(const char *& in, char (&out)[N])
{
return ReadDomainNameComponent(in, out, N);
}
#endif

NetworkCommissioning::otScanResponseIterator<NetworkCommissioning::ThreadScanResponse> mScanResponseIter;
} // namespace

Expand Down Expand Up @@ -2320,29 +2343,8 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread<ImplClass>::FromOtDnsRespons
{
char protocol[chip::Dnssd::kDnssdProtocolTextMaxSize + 1];

if (strchr(serviceType, '.') == nullptr)
return CHIP_ERROR_INVALID_ARGUMENT;

// Extract from the <type>.<protocol>.<domain-name>. the <type> part.
size_t substringSize = strchr(serviceType, '.') - serviceType;
if (substringSize >= ArraySize(mdnsService.mType))
{
return CHIP_ERROR_INVALID_ARGUMENT;
}
Platform::CopyString(mdnsService.mType, substringSize + 1, serviceType);

// Extract from the <type>.<protocol>.<domain-name>. the <protocol> part.
const char * protocolSubstringStart = serviceType + substringSize + 1;

if (strchr(protocolSubstringStart, '.') == nullptr)
return CHIP_ERROR_INVALID_ARGUMENT;

substringSize = strchr(protocolSubstringStart, '.') - protocolSubstringStart;
if (substringSize >= ArraySize(protocol))
{
return CHIP_ERROR_INVALID_ARGUMENT;
}
Platform::CopyString(protocol, substringSize + 1, protocolSubstringStart);
ReturnErrorOnFailure(ReadDomainNameComponent(serviceType, mdnsService.mType));
ReturnErrorOnFailure(ReadDomainNameComponent(serviceType, protocol));

if (strncmp(protocol, "_udp", chip::Dnssd::kDnssdProtocolTextMaxSize) == 0)
{
Expand All @@ -2357,24 +2359,20 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread<ImplClass>::FromOtDnsRespons
mdnsService.mProtocol = chip::Dnssd::DnssdServiceProtocol::kDnssdProtocolUnknown;
}

mdnsService.mInterface = Inet::InterfaceId::Null();
mdnsService.mSubTypeSize = 0;
mdnsService.mTextEntrySize = 0;

// Check if SRV record was included in DNS response.
if (error != OT_ERROR_NOT_FOUND)
// If not, return partial information about the service and exit early.
if (error != OT_ERROR_NONE)
{
if (strchr(serviceInfo.mHostNameBuffer, '.') == nullptr)
return CHIP_ERROR_INVALID_ARGUMENT;

// Extract from the <hostname>.<domain-name>. the <hostname> part.
substringSize = strchr(serviceInfo.mHostNameBuffer, '.') - serviceInfo.mHostNameBuffer;
if (substringSize >= ArraySize(mdnsService.mHostName))
{
return CHIP_ERROR_INVALID_ARGUMENT;
}
Platform::CopyString(mdnsService.mHostName, substringSize + 1, serviceInfo.mHostNameBuffer);

mdnsService.mPort = serviceInfo.mPort;
return CHIP_NO_ERROR;
}

mdnsService.mInterface = Inet::InterfaceId::Null();
const char * host = serviceInfo.mHostNameBuffer;
ReturnErrorOnFailure(ReadDomainNameComponent(host, mdnsService.mHostName));
mdnsService.mPort = serviceInfo.mPort;

// Check if AAAA record was included in DNS response.

Expand Down

0 comments on commit cfb6b1d

Please sign in to comment.