Skip to content

do_syslog_sudo: clamp the split length to avoid an infinite loop - #569

Merged
millert merged 1 commit into
sudo-project:mainfrom
iefa-m:do-syslog-sudo-maxlen-loop
Oct 20, 2026
Merged

millert merged 1 commit into
sudo-project:mainfrom
iefa-m:do-syslog-sudo-maxlen-loop

Conversation

@iefa-m

@iefa-m iefa-m commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

do_syslog_sudo() splits a long log line into several syslog(3) calls, computing the per-line budget as maxlen = syslog_maxlen - (strlen(fmt) - 5 + strlen(submituser)). On sudo_logsrvd the submit user comes straight from the client's AcceptMessage (evlog_new() fills evlog->submituser from the submituser key), so its length is attacker controlled. When the escaped submit user is exactly syslog_maxlen minus the continuation overhead (23 bytes, so 937 at the default syslog_maxlen of 960) the continuation maxlen works out to 0. memrchr(p, ' ', 0) returns NULL, tmp is set to p, the line is truncated to empty and p never advances, so the for loop spins forever emitting empty "(command continued)" records. It is reachable on the default logsrvd config (syslog, sudo format) with no TLS client certificate, and hangs the single event-loop daemon for every client.

Compute the overhead separately and clamp maxlen to a minimum of 1 at both the first-line and continuation sites, so the loop always advances. The same guard also avoids the size_t wrap when syslog_maxlen is smaller than the overhead. I confirmed the hang by replaying the split loop with the 937-byte submit user: unpacked it never advances p, and with the clamp it terminates. make check passes.

@millert millert left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is mitigated by 5e3b8b0, but it still seems like a good idea to guarantee the loop can make progress in all cases.

@millert
millert merged commit 0e2c376 into sudo-project:main Oct 20, 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.

2 participants