From 9b503bc77caf449d9a8a18a5c93754b6b0aaba62 Mon Sep 17 00:00:00 2001 From: permissionBRICK <40219477+permissionBRICK@users.noreply.github.com> Date: Sun, 12 Jul 2026 01:31:36 +0000 Subject: [PATCH] fix: validate NTP responses before use MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit handleNTPPacket() memcpy'd 80 bytes from packet.data() unconditionally, over-reading the stack when a short or garbage UDP datagram arrives (NTP responses are 48 bytes). The derived epoch was applied without any sanity check — a corrupt packet silently sets an arbitrary system time, which drives sunrise/sunset communication windows and scheduled reboots. Changes: - Reject packets shorter than 48 bytes (NTP_PACKET_SIZE) before reading - Reduce buffer to NTP_PACKET_SIZE and copy only the needed bytes - Validate NTP timestamp >= 3786825600 (2020-01-01 in NTP era) to reject corrupt values and prevent unsigned underflow in epoch subtraction - On rejection, return early without clearing the NTP timeout so the existing retry mechanism fires naturally (no stall) Lower bound rationale: 3786825600 is 2020-01-01 expressed in NTP seconds (seconds since 1900). This is well before any ahoy firmware release; any NTP time before this is unambiguously corrupt. Checking secsSince1900 directly (rather than the derived epoch) also prevents unsigned underflow when secsSince1900 < 2208988800 (the Unix epoch offset). --- src/network/AhoyNetwork.h | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/network/AhoyNetwork.h b/src/network/AhoyNetwork.h index 1945fbb4..ab3343dd 100644 --- a/src/network/AhoyNetwork.h +++ b/src/network/AhoyNetwork.h @@ -247,9 +247,13 @@ class AhoyNetwork { } void handleNTPPacket(AsyncUDPPacket packet) { - char buf[80]; + if(packet.length() < NTP_PACKET_SIZE) { + DPRINTLN(DBG_WARN, F("NTP packet too short")); + return; + } - memcpy(buf, packet.data(), sizeof(buf)); + uint8_t buf[NTP_PACKET_SIZE]; + memcpy(buf, packet.data(), NTP_PACKET_SIZE); unsigned long highWord = word(buf[40], buf[41]); unsigned long lowWord = word(buf[42], buf[43]); @@ -258,9 +262,19 @@ class AhoyNetwork { // this is NTP time (seconds since Jan 1 1900): unsigned long secsSince1900 = highWord << 16 | lowWord; + // 3786825600 = 2020-01-01 in NTP era (2208988800 + 1577836800); + // reject anything before that to guard against corrupt packets + // and prevent unsigned underflow in the epoch subtraction + if(secsSince1900 < 3786825600UL) { + DPRINTLN(DBG_WARN, F("NTP epoch out of range")); + return; + } + + uint32_t epoch = secsSince1900 - 2208988800UL; + mUdp.close(); - mNtpTimeoutSec = 0; // clear timeout - mOnTimeCB(secsSince1900 - 2208988800UL); + mNtpTimeoutSec = 0; + mOnTimeCB(epoch); } protected: