Repository navigation
Conversation
The JSON event log callback copied every InfoMessage key from the client after format_json() had already written uuid, server_time, submit_time, peeraddr, iolog_path, iolog_offset and reason. A client could send its own peeraddr, uuid or iolog_path and the duplicate key, which most JSON readers resolve to the last value, replaced the value recorded by the server. Skip those keys with a warning.
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.
With
log_format = json,store_accept_local()andstore_reject_local()pass the client'sInfoMessagelist tologsrvd_json_log_cb(). That callback writes every key into the event object afterformat_json()has already written the keys sudo_logsrvd sets itself:uuid,server_time,submit_time,peeraddr,iolog_path,iolog_offsetandreason. Nothing checks the client's key names, so a client that sendspeeraddr=10.9.9.9,uuid=00000000-...oriolog_path=/var/log/sudo-io/forgedin its AcceptMessage gets a second copy of each key in the log. Most JSON readers (Pythonjson, jq, typical SIEM ingest) keep the last value, so the record shows the address, UUID and I/O log path the client chose instead of the ones the server recorded. The real sudo client and sudo_sendlog never send these keys.The fix skips those reserved keys in the callback and logs a warning. The callback is the only place that writes client keys into the event, and it only runs for accept and reject events. I reproduced it by running sudo_logsrvd (built from main, JSON logfile) with a small protobuf client that sends the keys above. Before the patch the event has two
peeraddr/uuid/iolog_pathentries andjson.load()returns10.9.9.9. After the patch only the server's values are written and the daemon warnsprotocol error: reserved key peeraddr.make checkpasses.