From 8a3e6c6cec10cd03548bb1a4a341969f9abf110b Mon Sep 17 00:00:00 2001
From: Psykii22 <189542486+Psykii22@users.noreply.github.com>
Date: Fri, 30 Jan 2026 20:21:19 +0530
Subject: [PATCH] Fix #236: Prevent fabricated epoch timestamps before NTP
 synchronization

This commit implements a two-layer security fix to address the vulnerability
where getEpochTime() returned fabricated values (device uptime) before NTP
synchronization completed.

Security Issue:
- Before fix: getEpochTime() returned millis()/1000 (uptime values like 14, 16, 17...)
- These fabricated values could be mistaken for valid epoch timestamps
- Created authentication bypass vulnerabilities in security-sensitive applications
- Systems using timestamps for auth, replay protection, or HMAC were at risk

Layer 1 - Core Fix (getEpochTime):
- Now returns 0 when _lastUpdate == 0 (before synchronization)
- Aligns with ESP32 native SNTP behavior (time(nullptr) returns 0 until sync)
- Prevents fabricated values from being generated
- Fail-safe protection even if developers forget to check sync status

Layer 2 - Enhanced Validation (isTimeValid):
- New method that checks BOTH synchronization AND epoch reasonableness
- Returns true only if time is set AND epoch > 946684800 (Jan 1, 2000)
- Recommended for security-sensitive applications
- Catches edge cases where time might be "set" but invalid

Backward Compatibility:
- Existing isTimeSet() method unchanged
- Code checking isTimeSet() before using getEpochTime() continues to work
- New isTimeValid() is optional but recommended for security
- No breaking changes to existing functionality

Usage:
  // For UI/display:
  if (timeClient.isTimeSet()) {
    Serial.println(timeClient.getFormattedTime());
  }

  // For security operations:
  if (timeClient.isTimeValid()) {
    generateAuthToken(timeClient.getEpochTime());
  }

Fixes #236
---
 NTPClient.cpp | 20 ++++++++++++++++++++
 NTPClient.h   | 12 ++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/NTPClient.cpp b/NTPClient.cpp
index b435855..c5c8c1b 100755
--- a/NTPClient.cpp
+++ b/NTPClient.cpp
@@ -130,7 +130,27 @@ bool NTPClient::isTimeSet() const {
   return (this->_lastUpdate != 0); // returns true if the time has been set, else false
 }
 
+bool NTPClient::isTimeValid() const {
+  // First check if time has been set at all
+  if (!isTimeSet()) {
+    return false;
+  }
+  
+  // Then check if the epoch is reasonable (after Jan 1, 2000)
+  // This catches edge cases where time might be "set" but invalid
+  // 946684800 = Jan 1, 2000, 00:00:00 UTC
+  unsigned long epoch = getEpochTime();
+  return (epoch > 946684800);
+}
+
 unsigned long NTPClient::getEpochTime() const {
+  // Return 0 if time has not been synchronized yet
+  // This prevents returning fabricated uptime values (millis()/1000)
+
+  if (this->_lastUpdate == 0) {
+    return 0;
+  }
+  
   return this->_timeOffset + // User offset
          this->_currentEpoc + // Epoch returned by the NTP server
          ((millis() - this->_lastUpdate) / 1000); // Time since last update
diff --git a/NTPClient.h b/NTPClient.h
index a31d32f..1482a7e 100755
--- a/NTPClient.h
+++ b/NTPClient.h
@@ -81,6 +81,18 @@ class NTPClient {
      */
     bool isTimeSet() const;
 
+    /**
+     * Checks if the time is both set AND valid (reasonable epoch value).
+     * This is a stronger check than isTimeSet() - it verifies the epoch is
+     * greater than a minimum threshold (Jan 1, 2000) to catch edge cases.
+     *
+     * Recommended for security-sensitive applications that need to ensure
+     * time is not only synchronized but also plausible.
+     *
+     * @return true if time is set and epoch > 946684800 (Jan 1, 2000), else false
+     */
+    bool isTimeValid() const;
+
     int getDay() const;
     int getHours() const;
     int getMinutes() const;