Bug 1056 - use_pty sometimes writes stderr to the wrong PTY
use_pty sometimes writes stderr to the wrong PTY
Status: RESOLVED FIXED
Product: Sudo
Classification: Unclassified
Component: Sudo
1.9.14
PC Linux
: low normal
Assigned To: Todd C. Miller
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2023-07-25 15:26 MDT by Pierre Bourdon
Modified: 2023-07-26 20:06 MDT (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Pierre Bourdon 2023-07-25 15:26:15 MDT
Repro testcase using ssh and expect since this was the easiest way I found to muck around with PTYs:

#! /usr/bin/env expect -f
spawn ssh -t delroth@myhost "exec <\$SSH_TTY >\$SSH_TTY 2>/dev/tty0; echo ssh_tty: \$SSH_TTY; sudo -u nginx sudo -n -u root true || echo end"
expect "end"

Where delroth@myhost is a user which can use sudo NOPASSWD (so the outer sudo invocation works) but nginx@myhost is a user which cannot use sudo (so the inner sudo invocation fails).

Output:
spawn ssh -t delroth@sunny exec <$SSH_TTY >$SSH_TTY 2>/dev/tty0; echo ssh_tty: $SSH_TTY; sudo -u nginx sudo -n -u root true || echo end
ssh_tty: /dev/pts/1
sudo: a password is required
end

As you can see, $SSH_TTY is /dev/pts/1, which is not /dev/tty0. stderr is redirected to /dev/tty0, but "sudo: a password is required" still shows up on /dev/pts/1.

This happens only in very specific situations. Things that don't trigger the same issue:
- Redirecting stderr to a file instead of another TTY/PTY.
- Redirecting stderr to /dev/null.
- Not nesting sudo.

This was originally discovered by the NixOS integration test suite for sudo when attempting to update to 1.9.14. The integration test runner splits stdout and stderr into two separate virtual (qemu) TTY devices, and the tests started failing on 1.9.14 because stderr unexpectedly showed up on stdout. https://github.com/NixOS/nixpkgs/pull/240681

(This is likely not a new issue in 1.9.14, but use_pty wasn't the default before. I have not tried to bisect this.)
Comment 1 Todd C. Miller 2023-07-25 17:41:13 MDT
Sudo checks whether std{in,out,err} is a tty and, if not, interposes itself using a pipe.  However, it should probably check that the fd is connected to the user's /dev/tty instead of just being a terminal.
Comment 2 Todd C. Miller 2023-07-26 19:45:09 MDT
This has been fixed by https://www.sudo.ws/repos/sudo/rev/42838100b526
Comment 3 Pierre Bourdon 2023-07-26 19:50:19 MDT
Thank you! I've just tested the patch and confirmed the NixOS integration tests are now passing.

Are you planning to include this in a p4 release in the near future or leaving this bugfix for .15? Just wondering if I should wait a bit more or just backport this patch on top of p3.
Comment 4 Pierre Bourdon 2023-07-26 20:03:27 MDT
(In case this is useful for your decision process: it's ~0 work for us - NixOS - to carry the backported patch on top of p3. So if it's a significant amount of effort to do another patch release, please don't bother just for us, especially since this is a fairly minor bug.)
Comment 5 Todd C. Miller 2023-07-26 20:06:04 MDT
I don't think this warrants a 1.9.14p4 release so I'm currently planning to wait until 1.9.15.  If it ends up affecting others I'll revisit that decision.
Comment 6 Pierre Bourdon 2023-07-26 20:06:55 MDT
Makes sense, I'll proceed with pushing p3 + this patch on our side then. Thanks again for the quick turnaround!