[iefa-m]

handle_server_error() sets closure->errstr to msg->u.error, a string owned by the ServerMessage that handle_server_message() just unpacked. That function frees the message with server_message__free_unpacked() as soon as the handler returns false, and relay_server_msg_cb() then passes the now dangling closure->errstr to schedule_error_message(), which packs it into the error ServerMessage sent back to the client. The error and abort strings come straight off the wire from the relay peer, so in relay mode an error or abort message makes sudo_logsrvd read freed heap and copy those bytes into its reply to the client. This came in with fcc315f, which moved the scheduling out of the handler but kept the pointer.

Every other closure->errstr in the daemon is a static translated string and errstr is never freed, so do the same here and leave the relay's own text to the debug log just above. Confirmed with AddressSanitizer by unpacking an ERROR ServerMessage, aliasing u.error, freeing the message, then formatting the error reply: a heap-use-after-free read of the freed string, gone after the change. make check passes.

[millert]

It's a little unfortunate that we can't pass the relay server's error message to the client but this keeps the lifetime of errstr consistent which is more important.

[iefa-m]

Agreed, losing the relay's text is a shame, but it still ends up in the debug log so it isn't gone entirely. Thanks for merging this.