From 1e29d9463e23d0a9ef2841eb3b92d69a0881ba60 Mon Sep 17 00:00:00 2001
From: Frank <91616163+softhack007@users.noreply.github.com>
Date: Mon, 20 Nov 2023 12:24:34 +0100
Subject: [PATCH] validate NTP responses (fixes #3515)

* 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)
---
 wled00/const.h |  3 ++-
 wled00/ntp.cpp | 27 ++++++++++++++++++++++++++-
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/wled00/const.h b/wled00/const.h
index 25fe43773..f1293fd95 100644
--- a/wled00/const.h
+++ b/wled00/const.h
@@ -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
diff --git a/wled00/ntp.cpp b/wled00/ntp.cpp
index c9c4874b9..85bdcc7f8 100644
--- a/wled00/ntp.cpp
+++ b/wled00/ntp.cpp
@@ -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();
     }
@@ -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);