Skip to content

ntptime: Reject short NTP replies. - #1151

Merged
dpgeorge merged 1 commit into
micropython:masterfrom
pablogventura:ntptime-validate-response
Sep 11, 2026
Merged

ntptime: Reject short NTP replies.#1151
dpgeorge merged 1 commit into
micropython:masterfrom
pablogventura:ntptime-validate-response

Conversation

@pablogventura

@pablogventura pablogventura commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Fixes ntptime spoof or bad response shape #1136: time() used recv(48) and unpacked msg[40:44] with no length check. A short datagram could raise a confusing error or yield a bad timestamp.
  • Require len(msg) >= 48, otherwise OSError(-1) (same family as a socket timeout). Still uses recv(48).
  • Do not compare the UDP source address to the send target (can differ in complex network topologies).
  • Kiss of Death / zero timestamps are unchanged (see ntptime: Does not handle KoD (Kiss of Death) resonse. #604).

Test plan

  • micropython/net/ntptime/test_ntptime.py (valid 48-byte reply, short payload), wired into package tests CI.

@pablogventura

Copy link
Copy Markdown
Contributor Author

ntptime.mpy (mpy-cross, compiled as ntptime.py):

Version bytes vs master
master 563 -
this commit 600 +37

@dpgeorge

Copy link
Copy Markdown
Member

Thanks for the PR.

I'm not sure it's wise to check the address of the UDP sender via recvfrom(). In complex network topologies it could be that the IP address differs between the sendto and recvfrom.

See eg this implementation which indeed does not check the address: https://github.com/vicharl/ntpclient/blob/master/ntpclient.c#L326-L333

Also might be worth seeing how this library handles that scenario: https://github.com/ntpsec/ntpsec

The other change to reject short replies seems fine.

@pablogventura
pablogventura force-pushed the ntptime-validate-response branch from 266139a to 1aa3dfc Compare September 11, 2026 14:25
@pablogventura pablogventura changed the title ntptime: Reject short or spoofed NTP replies. ntptime: Reject short NTP replies. Sep 11, 2026
@pablogventura

Copy link
Copy Markdown
Contributor Author

Thanks @dpgeorge - dropped the recvfrom() source-address check and kept rejecting short replies with OSError(-1). Still uses recv(48).

Addressed in 1aa3dfc.

ntptime.mpy (mpy-cross, compiled as ntptime.py):

Version bytes vs master
master 563 -
previous on this branch 600 +37
this commit 584 +21

@dpgeorge dpgeorge left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for updating.

And add a few tests for ntptime.

Partially fixes micropython#1136.

Signed-off-by: Pablo Ventura <pablogventura@gmail.com>
@dpgeorge
dpgeorge force-pushed the ntptime-validate-response branch from 1aa3dfc to 9ec1830 Compare September 11, 2026 14:41
@dpgeorge
dpgeorge merged commit 9ec1830 into micropython:master Sep 11, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ntptime spoof or bad response shape

2 participants