From c67c80e93efb13662f2884055763a68b99d37f10 Mon Sep 17 00:00:00 2001 From: Me No Dev Date: Wed, 27 Jul 2016 16:42:06 +0300 Subject: [PATCH 1/5] Optimize MDNS to prevent overflow and endless loop --- libraries/ESP8266mDNS/ESP8266mDNS.cpp | 50 +++++++++++++++++++-------- 1 file changed, 35 insertions(+), 15 deletions(-) diff --git a/libraries/ESP8266mDNS/ESP8266mDNS.cpp b/libraries/ESP8266mDNS/ESP8266mDNS.cpp index 71dca73260..b29e59bde3 100644 --- a/libraries/ESP8266mDNS/ESP8266mDNS.cpp +++ b/libraries/ESP8266mDNS/ESP8266mDNS.cpp @@ -493,11 +493,11 @@ void MDNSResponder::_parsePacket(){ return; } - int numAnswers = packetHeader[3]; + int numAnswers = packetHeader[3] + packetHeader[5]; // Assume that the PTR answer always comes first and that it is always accompanied by a TXT, SRV, AAAA (optional) and A answer in the same packet. - if (numAnswers < 4) { + if (numAnswers < 4 || numAnswers > 5) { #ifdef MDNS_DEBUG_RX - Serial.println("Expected a packet with 4 answers, returning"); + Serial.println("Expected a packet with 4 or 5 answers, returning"); #endif _conn->flush(); return; @@ -510,11 +510,12 @@ void MDNSResponder::_parsePacket(){ bool serviceMatch = false; MDNSAnswer *answer; uint8_t partsCollected = 0; + uint8_t stringsRead = 0; // Clear answer list if (_newQuery) { - int numAnswers = _getNumAnswers(); - for (int n = numAnswers - 1; n >= 0; n--) { + int oldAnswers = _getNumAnswers(); + for (int n = oldAnswers - 1; n >= 0; n--) { answer = _getAnswerFromIdx(n); os_free(answer->hostname); os_free(answer); @@ -526,13 +527,21 @@ void MDNSResponder::_parsePacket(){ while (numAnswers--) { // Read name + stringsRead = 0; do { + if(stringsRead > 3){ +#ifdef MDNS_DEBUG_RX + Serial.println("failed to read the response name"); +#endif + _conn->flush(); + return; + } tmp8 = _conn_read8(); if (tmp8 & 0xC0) { // Compressed pointer (not supported) tmp8 = _conn_read8(); break; } - if (tmp8 == 0x00) { // �nd of name + if (tmp8 == 0x00) { // End of name break; } _conn_readS(serviceName, tmp8); @@ -540,7 +549,7 @@ void MDNSResponder::_parsePacket(){ #ifdef MDNS_DEBUG_RX Serial.printf(" %d ", tmp8); for (int n = 0; n < tmp8; n++) { - Serial.printf("%02x ", serviceName[n]); + Serial.printf("%c", serviceName[n]); } Serial.println(); #endif @@ -552,6 +561,7 @@ void MDNSResponder::_parsePacket(){ #endif } } + stringsRead++; } while (true); uint16_t answerType = _conn_read16(); // Read type @@ -559,6 +569,14 @@ void MDNSResponder::_parsePacket(){ uint32_t answerTtl = _conn_read32(); // Read ttl uint16_t answerRdlength = _conn_read16(); // Read rdlength + if(answerRdlength > 255){ +#ifdef MDNS_DEBUG_RX + Serial.println("Data len too long!"); +#endif + _conn->flush(); + return; + } + #ifdef MDNS_DEBUG_RX Serial.printf("type: %04x rdlength: %d\n", answerType, answerRdlength); #endif @@ -567,8 +585,9 @@ void MDNSResponder::_parsePacket(){ partsCollected |= 0x01; _conn_readS(hostName, answerRdlength); // Read rdata #ifdef MDNS_DEBUG_RX + Serial.printf("PTR %d ", answerRdlength); for (int n = 0; n < answerRdlength; n++) { - Serial.printf("%02x ", hostName[n]); + Serial.printf("%c", hostName[n]); } Serial.println(); #endif @@ -578,8 +597,9 @@ void MDNSResponder::_parsePacket(){ partsCollected |= 0x02; _conn_readS(hostName, answerRdlength); // Read rdata #ifdef MDNS_DEBUG_RX + Serial.printf("TXT %d ", answerRdlength); for (int n = 0; n < answerRdlength; n++) { - Serial.printf("%02x ", hostName[n]); + Serial.printf("%c", hostName[n]); } Serial.println(); #endif @@ -601,7 +621,7 @@ void MDNSResponder::_parsePacket(){ _conn_readS(answerHostName, tmp8); answerHostName[tmp8] = '\0'; #ifdef MDNS_DEBUG_RX - Serial.printf(" %d ", tmp8); + Serial.printf("SRV %d ", tmp8); for (int n = 0; n < tmp8; n++) { Serial.printf("%02x ", answerHostName[n]); } @@ -621,7 +641,7 @@ void MDNSResponder::_parsePacket(){ } else { #ifdef MDNS_DEBUG_RX - Serial.printf("Ignoring unsupported type %d\n", tmp8); + Serial.printf("Ignoring unsupported type %02x\n", tmp8); #endif for (int n = 0; n < answerRdlength; n++) (void)_conn_read8(); @@ -663,7 +683,7 @@ void MDNSResponder::_parsePacket(){ // PARSE REQUEST NAME - hostNameLen = _conn_read8(); + hostNameLen = _conn_read8() % 255; _conn_readS(hostName, hostNameLen); hostName[hostNameLen] = '\0'; @@ -685,7 +705,7 @@ void MDNSResponder::_parsePacket(){ } if(!serviceParsed){ - serviceNameLen = _conn_read8(); + serviceNameLen = _conn_read8() % 255; _conn_readS(serviceName, serviceNameLen); serviceName[serviceNameLen] = '\0'; @@ -718,7 +738,7 @@ void MDNSResponder::_parsePacket(){ } if(!protoParsed){ - protoNameLen = _conn_read8(); + protoNameLen = _conn_read8() % 255; _conn_readS(protoName, protoNameLen); protoName[protoNameLen] = '\0'; if(protoNameLen == 4 && protoName[0] == '_'){ @@ -740,7 +760,7 @@ void MDNSResponder::_parsePacket(){ if(!localParsed){ char localName[32]; - uint8_t localNameLen = _conn_read8(); + uint8_t localNameLen = _conn_read8() % 31; _conn_readS(localName, localNameLen); localName[localNameLen] = '\0'; tmp = _conn_read8(); From afa95cf033a01c03bd6245d5c2b37e22dfc22a57 Mon Sep 17 00:00:00 2001 From: Me No Dev Date: Wed, 27 Jul 2016 17:25:18 +0300 Subject: [PATCH 2/5] Handle better non-esp services --- libraries/ESP8266mDNS/ESP8266mDNS.cpp | 34 +++++++++++++++++---------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/libraries/ESP8266mDNS/ESP8266mDNS.cpp b/libraries/ESP8266mDNS/ESP8266mDNS.cpp index b29e59bde3..003e612f0a 100644 --- a/libraries/ESP8266mDNS/ESP8266mDNS.cpp +++ b/libraries/ESP8266mDNS/ESP8266mDNS.cpp @@ -61,7 +61,7 @@ extern "C" { //#define MDNS_DEBUG_ERR //#define MDNS_DEBUG_TX -//#define MDNS_DEBUG_RX +#define MDNS_DEBUG_RX #define MDNS_NAME_REF 0xC000 @@ -495,9 +495,9 @@ void MDNSResponder::_parsePacket(){ int numAnswers = packetHeader[3] + packetHeader[5]; // Assume that the PTR answer always comes first and that it is always accompanied by a TXT, SRV, AAAA (optional) and A answer in the same packet. - if (numAnswers < 4 || numAnswers > 5) { + if (numAnswers < 4) { #ifdef MDNS_DEBUG_RX - Serial.println("Expected a packet with 4 or 5 answers, returning"); + Serial.printf("Expected a packet with 4 or more answers, got %u\n", numAnswers); #endif _conn->flush(); return; @@ -529,13 +529,6 @@ void MDNSResponder::_parsePacket(){ // Read name stringsRead = 0; do { - if(stringsRead > 3){ -#ifdef MDNS_DEBUG_RX - Serial.println("failed to read the response name"); -#endif - _conn->flush(); - return; - } tmp8 = _conn_read8(); if (tmp8 & 0xC0) { // Compressed pointer (not supported) tmp8 = _conn_read8(); @@ -544,6 +537,13 @@ void MDNSResponder::_parsePacket(){ if (tmp8 == 0x00) { // End of name break; } + if(stringsRead > 3){ +#ifdef MDNS_DEBUG_RX + Serial.println("failed to read the response name"); +#endif + _conn->flush(); + return; + } _conn_readS(serviceName, tmp8); serviceName[tmp8] = '\0'; #ifdef MDNS_DEBUG_RX @@ -570,11 +570,15 @@ void MDNSResponder::_parsePacket(){ uint16_t answerRdlength = _conn_read16(); // Read rdlength if(answerRdlength > 255){ + if(answerType == MDNS_TYPE_TXT && answerRdlength < 1460){ + while(--answerRdlength) _conn->read(); + } else { #ifdef MDNS_DEBUG_RX - Serial.println("Data len too long!"); + Serial.printf("Data len too long! %u\n", answerRdlength); #endif - _conn->flush(); - return; + _conn->flush(); + return; + } } #ifdef MDNS_DEBUG_RX @@ -614,7 +618,9 @@ void MDNSResponder::_parsePacket(){ // Read hostname tmp8 = _conn_read8(); if (tmp8 & 0xC0) { // Compressed pointer (not supported) +#ifdef MDNS_DEBUG_RX Serial.println("Skipping compressed pointer"); +#endif tmp8 = _conn_read8(); } else { @@ -674,6 +680,8 @@ void MDNSResponder::_parsePacket(){ } answer->hostname = (char *)os_malloc(strlen(answerHostName) + 1); os_strcpy(answer->hostname, answerHostName); + _conn->flush(); + return; } } From 28f50a57ada764a27d7e7a2e96e2f12f6536389c Mon Sep 17 00:00:00 2001 From: Me No Dev Date: Wed, 27 Jul 2016 17:26:09 +0300 Subject: [PATCH 3/5] leave debug off --- libraries/ESP8266mDNS/ESP8266mDNS.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/ESP8266mDNS/ESP8266mDNS.cpp b/libraries/ESP8266mDNS/ESP8266mDNS.cpp index 003e612f0a..1108bff586 100644 --- a/libraries/ESP8266mDNS/ESP8266mDNS.cpp +++ b/libraries/ESP8266mDNS/ESP8266mDNS.cpp @@ -61,7 +61,7 @@ extern "C" { //#define MDNS_DEBUG_ERR //#define MDNS_DEBUG_TX -#define MDNS_DEBUG_RX +//#define MDNS_DEBUG_RX #define MDNS_NAME_REF 0xC000 From 282a0f2ec272f0809f28013817da6d7b746c1cb3 Mon Sep 17 00:00:00 2001 From: Me No Dev Date: Wed, 27 Jul 2016 17:38:54 +0300 Subject: [PATCH 4/5] better name resolution for external devices --- libraries/ESP8266mDNS/ESP8266mDNS.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/libraries/ESP8266mDNS/ESP8266mDNS.cpp b/libraries/ESP8266mDNS/ESP8266mDNS.cpp index 1108bff586..b1fb432287 100644 --- a/libraries/ESP8266mDNS/ESP8266mDNS.cpp +++ b/libraries/ESP8266mDNS/ESP8266mDNS.cpp @@ -512,6 +512,8 @@ void MDNSResponder::_parsePacket(){ uint8_t partsCollected = 0; uint8_t stringsRead = 0; + answerHostName[0] = '\0'; + // Clear answer list if (_newQuery) { int oldAnswers = _getNumAnswers(); @@ -588,6 +590,10 @@ void MDNSResponder::_parsePacket(){ if (answerType == MDNS_TYPE_PTR) { partsCollected |= 0x01; _conn_readS(hostName, answerRdlength); // Read rdata + if(hostName[answerRdlength-2] & 0xc0){ + memcpy(answerHostName, hostName, answerRdlength-2); + answerHostName[answerRdlength-2] = '\0'; + } #ifdef MDNS_DEBUG_RX Serial.printf("PTR %d ", answerRdlength); for (int n = 0; n < answerRdlength; n++) { From 41ebe39b5bde969add15bd5d44f6b3f6b489e3ec Mon Sep 17 00:00:00 2001 From: Me No Dev Date: Wed, 27 Jul 2016 19:06:31 +0300 Subject: [PATCH 5/5] strip name length (did not show in IDE) --- libraries/ESP8266mDNS/ESP8266mDNS.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/ESP8266mDNS/ESP8266mDNS.cpp b/libraries/ESP8266mDNS/ESP8266mDNS.cpp index b1fb432287..764264fc38 100644 --- a/libraries/ESP8266mDNS/ESP8266mDNS.cpp +++ b/libraries/ESP8266mDNS/ESP8266mDNS.cpp @@ -591,8 +591,8 @@ void MDNSResponder::_parsePacket(){ partsCollected |= 0x01; _conn_readS(hostName, answerRdlength); // Read rdata if(hostName[answerRdlength-2] & 0xc0){ - memcpy(answerHostName, hostName, answerRdlength-2); - answerHostName[answerRdlength-2] = '\0'; + memcpy(answerHostName, hostName+1, answerRdlength-3); + answerHostName[answerRdlength-3] = '\0'; } #ifdef MDNS_DEBUG_RX Serial.printf("PTR %d ", answerRdlength);