Skip to content

Commit

Permalink
validate NTP responses (fixes #3515)
Browse files Browse the repository at this point in the history
* purge old (not yet processes) NTP responses
* validate server responses before updating WLED time
* purge receive buffer when package is rejected (avoids mem leak on esp32)
  • Loading branch information
softhack007 committed Nov 20, 2023
1 parent 75f6a53 commit 1e29d94
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 2 deletions.
3 changes: 2 additions & 1 deletion wled00/const.h
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,8 @@
#define SUBPAGE_JS 254
#define SUBPAGE_WELCOME 255

#define NTP_PACKET_SIZE 48
#define NTP_PACKET_SIZE 48 // size of NTP recive buffer
#define NTP_MIN_PACKET_SIZE 48 // min expected size - NTP v4 allows for "extended information" appended to the standard fields

//maximum number of rendered LEDs - this does not have to match max. physical LEDs, e.g. if there are virtual busses
#ifndef MAX_LEDS
Expand Down
27 changes: 26 additions & 1 deletion wled00/ntp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,9 @@ void handleNetworkTime()
{
if (millis() - ntpPacketSentTime > 10000)
{
#ifdef ARDUINO_ARCH_ESP32 // I had problems using udp.flush() on 8266
while (ntpUdp.parsePacket() > 0) ntpUdp.flush(); // flush any existing packets
#endif
sendNTPPacket();
ntpPacketSentTime = millis();
}
Expand Down Expand Up @@ -239,16 +242,38 @@ void sendNTPPacket()
ntpUdp.endPacket();
}

static bool isValidNtpResponse(byte * ntpPacket) {
// Perform a few validity checks on the packet
// based on https://github.com/taranais/NTPClient/blob/master/NTPClient.cpp
if((ntpPacket[0] & 0b11000000) == 0b11000000) return false; //reject LI=UNSYNC
// if((ntpPacket[0] & 0b00111000) >> 3 < 0b100) return false; //reject Version < 4
if((ntpPacket[0] & 0b00000111) != 0b100) return false; //reject Mode == Server
if((ntpPacket[1] < 1) || (ntpPacket[1] > 15)) return false; //reject invalid Stratum
if( ntpPacket[16] == 0 && ntpPacket[17] == 0 &&
ntpPacket[18] == 0 && ntpPacket[19] == 0 &&
ntpPacket[20] == 0 && ntpPacket[21] == 0 &&
ntpPacket[22] == 0 && ntpPacket[23] == 0) //reject ReferenceTimestamp == 0
return false;

return true;
}

bool checkNTPResponse()
{
int cb = ntpUdp.parsePacket();
if (!cb) return false;
if (cb < NTP_MIN_PACKET_SIZE) {
#ifdef ARDUINO_ARCH_ESP32 // I had problems using udp.flush() on 8266
if (cb > 0) ntpUdp.flush(); // this avoids memory leaks on esp32
#endif
return false;
}

uint32_t ntpPacketReceivedTime = millis();
DEBUG_PRINT(F("NTP recv, l="));
DEBUG_PRINTLN(cb);
byte pbuf[NTP_PACKET_SIZE];
ntpUdp.read(pbuf, NTP_PACKET_SIZE); // read the packet into the buffer
if (!isValidNtpResponse(pbuf)) return false; // verify we have a valid response to client

Toki::Time arrived = toki.fromNTP(pbuf + 32);
Toki::Time departed = toki.fromNTP(pbuf + 40);
Expand Down

0 comments on commit 1e29d94

Please sign in to comment.