diff --git a/.github/.cSpellWords.txt b/.github/.cSpellWords.txt index 56e2938c9..d45938573 100644 --- a/.github/.cSpellWords.txt +++ b/.github/.cSpellWords.txt @@ -21,6 +21,7 @@ amsdu ANAR ANARBMCR ANCEN +ANCOUNT ANEG ANEGCAPABLE ANEGCOMPLETE diff --git a/source/FreeRTOS_DNS_Parser.c b/source/FreeRTOS_DNS_Parser.c index 90a7f8517..7e79c8cfb 100644 --- a/source/FreeRTOS_DNS_Parser.c +++ b/source/FreeRTOS_DNS_Parser.c @@ -693,10 +693,16 @@ BaseType_t xReturn = pdTRUE; const DNSAnswerRecord_t * pxDNSAnswerRecord; IPv46_Address_t xIP_Address; + uint16_t usMaxAnswers = pxSet->usAnswers; struct freertos_addrinfo * pxNewAddress = NULL; - for( x = 0U; x < pxSet->usAnswers; x++ ) + if( usMaxAnswers > ipconfigDNS_CACHE_ADDRESSES_PER_ENTRY ) + { + usMaxAnswers = ipconfigDNS_CACHE_ADDRESSES_PER_ENTRY; + } + + for( x = 0U; x < usMaxAnswers; x++ ) { BaseType_t xDoAccept = pdFALSE; diff --git a/test/unit-test/FreeRTOS_DNS_Parser/FreeRTOS_DNS_Parser_utest.c b/test/unit-test/FreeRTOS_DNS_Parser/FreeRTOS_DNS_Parser_utest.c index ac18be894..8ca8a7578 100644 --- a/test/unit-test/FreeRTOS_DNS_Parser/FreeRTOS_DNS_Parser_utest.c +++ b/test/unit-test/FreeRTOS_DNS_Parser/FreeRTOS_DNS_Parser_utest.c @@ -3721,6 +3721,57 @@ void test_parseDNSAnswer_recordstored_gt_count2( void ) TEST_ASSERT_EQUAL( 80, uxBytesRead ); } +/** + * @brief Verify that parseDNSAnswer bounds answer parsing to + * ipconfigDNS_CACHE_ADDRESSES_PER_ENTRY even when xDoStore is pdFALSE + * for an unsolicited reply. + * + * This ensures an attacker-controlled ANCOUNT does not cause the parser + * to iterate over an excessive number of advertised answers. + */ +void test_parseDNSAnswer_unsolicited_reply_loop_bounded( void ) +{ + BaseType_t ret; + DNSMessage_t pxDNSMessageHeader; + char pucByte[ 300 ]; + size_t uxBytesRead = 0; + ParseSet_t xSet = { 0 }; + struct freertos_addrinfo * pxAddressInfo = NULL; + int index = 0; + uint16_t i; + + memset( pucByte, 0x00, sizeof( pucByte ) ); + + /* Place a valid name field for each expected iteration so that + * DNS_SkipNameField succeeds and usChar2u16 is reached every time. */ + for( i = 0U; i < ( uint16_t ) ipconfigDNS_CACHE_ADDRESSES_PER_ENTRY; i++ ) + { + pucByte[ index ] = 38; + strcpy( pucByte + index + 1, "FreeRTOSbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" ); + index += 40; + } + + xSet.pxDNSMessageHeader = &pxDNSMessageHeader; + xSet.pucByte = pucByte; + xSet.uxSourceBytesRemaining = sizeof( pucByte ); + xSet.xDoStore = pdFALSE; + xSet.usNumARecordsStored = 0; + xSet.usAnswers = 0xFFFFU; /* Simulated attacker-controlled ANCOUNT. */ + + /* Expect exactly ipconfigDNS_CACHE_ADDRESSES_PER_ENTRY calls to + * usChar2u16. If the loop exceeds the cap, CMock fails on an + * unexpected call; if it falls short, CMock fails on an unmet + * expectation. */ + for( i = 0U; i < ( uint16_t ) ipconfigDNS_CACHE_ADDRESSES_PER_ENTRY; i++ ) + { + usChar2u16_ExpectAnyArgsAndReturn( dnsTYPE_A_HOST ); + } + + ret = parseDNSAnswer( &xSet, &pxAddressInfo, &uxBytesRead ); + + TEST_ASSERT_EQUAL( 0, ret ); +} + /** * @brief ensures that when the number of entries is larger than the configured * cache address entries, not packet is sent over the network