ntpd: bound remote config error accumulation - #31
Open
carrerasdarren-cell wants to merge 1 commit into
Open
Conversation
snprintf returns the number of bytes that would have been written, so repeated remote parser diagnostics can advance err_pos beyond err_msg and corrupt adjacent daemon state. Use the existing bounded string-buffer formatter and derive err_pos from its cursor. This is a modernized port of NTPsec commit a619d39ac2b6d3b435edd2f6f527c7cc81f78d02 by Gary E. Miller.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Bound remote parser error accumulation with the existing
xsbprintf()helper.The previous code advanced
remote_config.err_posbysnprintf()'swould-have-written length, so a truncated diagnostic could move the cursor past
the 1024-byte
err_msgarray.Security impact
A client with a valid mode 6 control key can submit many remote-only parser
diagnostics in one authenticated
:configrequest. In a local sanitizer build,enablefollowed by 70mode7tokens advanced the cursor to index 1063,overwrote adjacent daemon state, and ended in an AddressSanitizer crash.
The patch appends only complete messages that fit and derives
err_posfrom thebounded helper cursor. The error count is still accurate even when the response
buffer is full.
This is a transparent, modernized port of
NTPsec commit a619d39ac2b6d3b435edd2f6f527c7cc81f78d02,
authored by Gary E. Miller in 2018. I am not claiming the root cause as a new
discovery.
Verification
ntpdpasses.test-sbprintfsuite passes: 8 tests, 0 failures.and daemon crash under ASan/UBSan.
remains alive, and handles a second authenticated command successfully.