[Git][NTPsec/ntpsec][master] libparse/clk_trimtsip.c: fix copy-paste bug in TSIP week unwrap

[Matt Selsky (@selsky)]

Matt Selsky pushed to branch master at NTPsec / ntpsec


Commits:
b6c8155e by Matt Selsky at 2026-10-02T02:39:35-04:00
libparse/clk_trimtsip.c: fix copy-paste bug in TSIP week unwrap

t_week (current GPS week) and t_weekleap (week of the next scheduled
leap-second correction) are distinct fields. When t_week needed epoch
unwrap, the code set it from t_weekleap+GPSWEEKS instead of
t_week+GPSWEEKS - introduced in d21c57eec (2015-03-19), a mechanical
type-cast cleanup that got the adjacent near-identical
t_weekleap/t_week blocks crossed. Coverity correctly flagged this as
a copy-paste error in b1fa16c58 (2015-11-04), but the finding was
suppressed rather than investigated ("More Coverity cleanup. No logic
changes.").

Impact - t_week's only consumer in this file is the leap-imminent
check (t_weekleap - t_week) < 5, which decides whether to raise the
driver's leap-second warning (ADDSECOND/DELSECOND):

- The bug does not affect the time value this refclock reports -
  only that advisory flag.
- Since t_weekleap is already unwrapped by this point, the buggy line
  sets t_week = t_weekleap + GPSWEEKS, so t_weekleap - t_week is
  exactly -1024 (not close to zero) - and -1024 < 5 unconditionally,
  every time. A server using the Trimble TSIP refclock would tell
  every client polling it, via the standard NTP leap-indicator bits,
  that a leap second is imminent as soon as the receiver has any leap
  correction queued at all - weeks or months before the intended
  ~5-week window, deterministically, not just occasionally missing
  the window.
- GPSWRAP is 990 of a 1024-week range, so the buggy branch executes
  on ~97% of CMD_RUTCPARAM messages, but the effect is only visible
  when the receiver also has a genuinely pending leap-second change
  (tlsf != tls) - an infrequent real-world event (most recently Dec
  2016). A given deployment could run for years without observing
  this, despite the code path itself being live almost always.
- Local hardware refclock parsing, not network-facing - a
  false-early leap-indicator bug, not a security issue.

Restore the original t_week += GPSWEEKS form (the pre-2015 code, and
what the fixed logic actually needs) rather than the explicit-cast
reassignment - the cast was never required (unsigned short += int is
well-defined, and the max resulting value here is 2013) and the
project doesn't build with -Wconversion, so it was solely a leftover
of that unrelated 2015 cleanup pass. Drop the now-stale coverity[]
suppression comment along with it.

- - - - -


1 changed file:

- libparse/clk_trimtsip.c


Changes:

=====================================
libparse/clk_trimtsip.c
=====================================
@@ -324,8 +324,7 @@ cvt_trimtsip(
 			    /* current week no */
 			    t->t_week = get_msb_ushort(&mb(18));
 			    if (t->t_week < GPSWRAP) {
-				    /* coverity[copy_paste_error] */
-				    t->t_week = (unsigned short)(t->t_weekleap + GPSWEEKS);
+				    t->t_week += GPSWEEKS;
 			    }
 
 			    lbp = (unsigned char *)&mb(14); /* last update time */



View it on GitLab: https://gitlab.com/NTPsec/ntpsec/-/commit/b6c8155e1c862bdfe79c381b8a96cdc6859c6f57

-- 
View it on GitLab: https://gitlab.com/NTPsec/ntpsec/-/commit/b6c8155e1c862bdfe79c381b8a96cdc6859c6f57
You're receiving this email because of your account on gitlab.com. Manage all notifications: https://gitlab.com/-/profile/notifications | Help: https://gitlab.com/help


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ntpsec.org/pipermail/vc/attachments/20261002/1836dc64/attachment.htm>