Browse Source

fix: validate NTP responses before use

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).
pull/1889/head
permissionBRICK 2 weeks ago
parent
commit
9b503bc77c
  1. 22
      src/network/AhoyNetwork.h

22
src/network/AhoyNetwork.h

@ -247,9 +247,13 @@ class AhoyNetwork {
} }
void handleNTPPacket(AsyncUDPPacket packet) { 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 highWord = word(buf[40], buf[41]);
unsigned long lowWord = word(buf[42], buf[43]); unsigned long lowWord = word(buf[42], buf[43]);
@ -258,9 +262,19 @@ class AhoyNetwork {
// this is NTP time (seconds since Jan 1 1900): // this is NTP time (seconds since Jan 1 1900):
unsigned long secsSince1900 = highWord << 16 | lowWord; 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(); mUdp.close();
mNtpTimeoutSec = 0; // clear timeout mNtpTimeoutSec = 0;
mOnTimeCB(secsSince1900 - 2208988800UL); mOnTimeCB(epoch);
} }
protected: protected:

Loading…
Cancel
Save