diff --git a/network/ntp.c b/network/ntp.c index 735d2d5..eb4ad19 100644 --- a/network/ntp.c +++ b/network/ntp.c @@ -115,10 +115,14 @@ static void apply_time(uint32_t unix_time) { static void ntp_recv_cb(void *arg, struct udp_pcb *pcb, struct pbuf *p, const ip_addr_t *addr, u16_t port) { + // M11: a rejected datagram is DROPPED, not treated as a sync failure. Every + // validation arm below frees its pbuf and returns without touching ntp_state, + // so ntp_sync()'s poll loop keeps waiting for the genuine reply until its + // deadline. A single spoofed packet from server_addr:123 can no longer abort + // the whole sync. Only the success path advances the state machine. if (p->tot_len < 48) { // use tot_len not len (L10 fix too) printf("[ntp] response too short\r\n"); pbuf_free(p); - ntp_state = NTP_STATE_FAILED; return; } @@ -129,7 +133,6 @@ static void ntp_recv_cb(void *arg, struct udp_pcb *pcb, struct pbuf *p, const ip // Validate source (belt-and-suspenders — udp_connect already filters) if (!ip_addr_cmp(addr, &server_addr) || port != NTP_PORT) { printf("[ntp] unexpected source\r\n"); - ntp_state = NTP_STATE_FAILED; return; } @@ -140,24 +143,20 @@ static void ntp_recv_cb(void *arg, struct udp_pcb *pcb, struct pbuf *p, const ip if (li == 3) { // LI=3: clock unsynchronised printf("[ntp] server clock unsynchronised\r\n"); - ntp_state = NTP_STATE_FAILED; return; } if (mode != 4) { // mode must be 4 (server) printf("[ntp] unexpected mode: %u\r\n", mode); - ntp_state = NTP_STATE_FAILED; return; } if (stratum == 0 || stratum > 15) { // 0=kiss-o-death, >15=invalid printf("[ntp] invalid stratum: %u\r\n", stratum); - ntp_state = NTP_STATE_FAILED; return; } // Verify origin timestamp (bytes 24-31) echoes our nonce if (memcmp(&buf[24], ntp_nonce, 8) != 0) { printf("[ntp] origin timestamp mismatch — possible replay\r\n"); - ntp_state = NTP_STATE_FAILED; return; } @@ -180,12 +179,10 @@ static void ntp_recv_cb(void *arg, struct udp_pcb *pcb, struct pbuf *p, const ip // seconds_since_1900 (H3) before either reaches the RTC. if (unix_time < BUILD_UNIX_TIME || unix_time - BUILD_UNIX_TIME > NTP_SANE_MAX_AGE_S) { printf("[ntp] timestamp outside sane band: %u\r\n", unix_time); - ntp_state = NTP_STATE_FAILED; return; } if (!rollback_check(unix_time)) { - ntp_state = NTP_STATE_FAILED; return; } diff --git a/serial/commands_system.c b/serial/commands_system.c index 4b92f9e..bdd3cac 100644 --- a/serial/commands_system.c +++ b/serial/commands_system.c @@ -64,11 +64,13 @@ void cmd_status(int argc, char **argv) { printf("ntp: not synced\r\n"); } - // Keys (admin only: key inventory is target-selection data) + // Keys (admin only: key inventory is target-selection data). Declared at + // function scope so the scrub below always runs, even on the non-admin path + // where the array stays zero-initialised. + static key_record_t keys[BACKUP_MAX_KEYS]; if (commands_is_admin()) { - static key_record_t keys[BACKUP_MAX_KEYS]; - int count = storage_key_list(keys, BACKUP_MAX_KEYS); - int enabled = 0, corrupt = 0; + int count = storage_key_list(keys, BACKUP_MAX_KEYS); + int enabled = 0, corrupt = 0; for (int i = 0; i < count; i++) { if (!keys[i].is_checksum_valid) corrupt++; diff --git a/test/fuzz_ntp.c b/test/fuzz_ntp.c index f00eb99..4e8aaab 100644 --- a/test/fuzz_ntp.c +++ b/test/fuzz_ntp.c @@ -149,6 +149,11 @@ bool wifi_is_connected(void) { /* --- helpers -------------------------------------------------------------- */ +/* A valid, deterministic source address (127.0.0.1). ntp_recv_cb's source check + * dereferences addr and compares it against server_addr, so the harness must feed + * a non-NULL addr that matches; reset_ntp_state() pins server_addr to it. */ +static const ip_addr_t g_src_addr = {.addr = 0x0100007fu}; + /* Reset ntp.c's file-static state so each scenario starts from a known point. */ static void reset_ntp_state(void) { synced = false; @@ -156,6 +161,7 @@ static void reset_ntp_state(void) { last_sync_monotonic_us = 0; rollback_budget_used_s = 0; ntp_state = NTP_STATE_IDLE; + server_addr = g_src_addr; /* so the source check accepts feed()'s addr */ g_applied_called = 0; g_applied_unix = 0; } @@ -189,7 +195,7 @@ static void feed(const uint8_t *bytes, size_t len) { static void make_ntp_packet(uint8_t pkt[48], uint32_t unix_time) { memset(pkt, 0, 48); pkt[0] = 0x1C; /* LI=0, VN=3, Mode=4 (server) */ - pkt[1] = 1; /* stratum 1 (valid: 1..15) */ + pkt[1] = 1; /* stratum: 1..15 valid (0 and >15 rejected) */ uint32_t seconds_since_1900 = unix_time + NTP_DELTA; pkt[40] = (uint8_t)(seconds_since_1900 >> 24); pkt[41] = (uint8_t)(seconds_since_1900 >> 16); @@ -230,14 +236,20 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { uint8_t pkt[48]; - /* Below the floor: rollback rejected, RTC untouched, state == FAILED. */ + /* M11: a rejected datagram is DROPPED without aborting the sync. We enter the + * reject in the WAITING state ntp_sync's poll loop holds and require the arm + * to leave it WAITING (not FAILED), so the loop keeps waiting for the genuine + * reply until its deadline. Below the floor: rollback rejected, RTC untouched, + * state still WAITING. */ + ntp_state = NTP_STATE_WAITING; make_ntp_packet(pkt, floor - 100u); feed(pkt, 48); assert(g_applied_called == 0); - assert(ntp_state == NTP_STATE_FAILED); + assert(ntp_state == NTP_STATE_WAITING); /* State unchanged by the reject (apply_time never ran), so the floor still - * holds. At/above the floor: accepted, RTC set to exactly that time. */ + * holds. At/above the floor: accepted, RTC set to exactly that time, and only + * now does the state machine advance to SUCCESS. */ make_ntp_packet(pkt, floor + 100u); feed(pkt, 48); assert(g_applied_called == 1);