OpenDoas icon indicating copy to clipboard operation
OpenDoas copied to clipboard

security vulnerability: TIOCSTI tty character injection

Open e00E opened this issue 1 year ago • 20 comments

This is written from the perspective of Arch Linux. Some man page quotes or commands might be different on other distributions. I am using the opendoas package.

Excerpt of the su manpage documenting the --pty argument:

Create a pseudo-terminal for the session. The independent terminal provides better security as the user does not share a terminal with the original session. This can be used to avoid TIOCSTI ioctl terminal injection and other security attacks against terminal file descriptors. The entire session can also be moved to the background (e.g., su --pty - username -c application &). If the pseudo-terminal is enabled, then su works as a proxy between the sessions (sync stdin and stdout).

OpenDoas is vulnerable to "security attacks against terminal file descriptors". I have have verified the TIOCSTI attack on my system and expect that it would still work if compiled from source.

OpenDoas does not have a --pty equivalent. There should at least be a clear warning in the manpage that running commands as another user (with a tty attached like when running interactively), allows that user to take over the current user running OpenDoas. Few people are aware of this.

https://ruderich.org/simon/notes/su-sudo-from-root-tty-hijacking has a good summary of the TIOCSTI issue. Note that this page highlights going from root to a non root user but the vulnerability also exists going from a non root user to another non root user.

https://github.com/containers/bubblewrap/issues/142 , https://news.ycombinator.com/item?id=17311808 and other links from there has more context.

Other tools like su and sudo have been vulnerable to this in the past and did not always have the --pty argument.

OpenBSD is not vulnerable to the TIOCSTI attack because they have (rightly, I wish Linux did too) disabled this syscall. Even so, the manpage quote mentions "other attacks" that I haven't seen documented clearly, which might still be serious.

e00E avatar Nov 05 '22 22:11 e00E

Note that the Linux kernel now supports disabling TIOCSTI as of version 6.2 (ref. It can be disabled at kernel compile time or afterwords with a sysctl.

ReillyBrogan avatar Feb 22 '23 03:02 ReillyBrogan

I would like to confirm that OpenDoas 6.8.2 is affected. To reproduce, I did this:

$ sudo tee -a /etc/doas.conf <<<"permit nopass ${USER} as nobody  # added by ${USER} on $(date -I)"
$ cat <<TIOCSTI_C_EOF | tee TIOCSTI.c
#include <sys/ioctl.h>

int main(void) {
  const char *text = "id\n";
  while (*text)
    ioctl(0, TIOCSTI, text++);
  return 0;
}
TIOCSTI_C_EOF
$ gcc -std=c99 -Wall -Wextra -pedantic -o /tmp/TIOCSTI TIOCSTI.c
$ doas -u nobody /tmp/TIOCSTI  # runs id(1) as ${USER}/${DOAS_USER} rather than nobody

The demo code is inspired by https://github.com/containers/bubblewrap/issues/142 .

hartwork avatar Mar 14 '23 01:03 hartwork

It turned out now — thanks to @jwilk's ttyjack — that it's not just TIOCSTI but also TIOCLINUX. I have code to block these two ioctls using seccomp libseccomp at antijack that I could re-use for a pull request adding protection to OpenDoas if welcome.

hartwork avatar Mar 14 '23 22:03 hartwork

Someone apparently requested a CVE and got CVE-2023-28339 for this.

hartwork avatar Mar 15 '23 22:03 hartwork

I don't really know what to do about that.

  • Using setsid(2) breaks the expected behavior.
  • Using seccomp leaves other platforms or kernels without seccomp vulnerable.
  • Implementing the whole pty stuff from scratch or copying it from sudo requires a lot more code.
    • Enabling it by default if any of the file descriptors are a tty.
    • Adding a new flag like like su's --pty leaves the default vulnerable and has the same drawbacks as just documenting it.
  • Documenting this and do nothing, this will just continue the cycle of people discovering this issue every few years when they happen to read the man page.

Duncaen avatar Mar 16 '23 17:03 Duncaen

* Using seccomp leaves other platforms or kernels without seccomp vulnerable.

Isn't protecting ~99% of users better than protecting none of them?

hyder365 avatar Mar 16 '23 17:03 hyder365

@Duncaen thanks for your reply, that's a nice summary. slicer69/doas is in the same situation and I believe it's not unlikely they will go for the PTY approach to have FreeBSD in the fixed boat. ptyas could be a good place to see a PTY-based protection implemented.

CC @slicer69

hartwork avatar Mar 16 '23 17:03 hartwork

PS: @Duncaen what about default PTY and a new option --no-pty? Would be ahead of sudo and su/util-linux regarding defaults then.

hartwork avatar Mar 16 '23 17:03 hartwork

@hyder365 it may be worth noting that both util-linux and GNU coreutils have reverted their mitigation based on libseccomp in the past:

  • https://github.com/util-linux/util-linux/commit/23f75093264aae5d58d61016cb1a29d8ebdfa157
  • https://github.com/coreutils/coreutils/commit/f5d7c0842ef7adc2be6e85f9ef66b35ebbbd6a61

At least util-linux probably did not have to consider support for FreeBSD.

hartwork avatar Mar 16 '23 18:03 hartwork

  • Documenting this and do nothing, this will just continue the cycle of people discovering this issue every few years when they happen to read the man page.

The issue should at least be documented like the su manpage does. What to do afterwards is is a different question. There is no need to wait with documenting until the bikeshedding for the best solution is done.

e00E avatar Mar 17 '23 14:03 e00E

@e00E bikeshedding is not the adequate term here and documentation on broken defaults has as little value as it has in sudo and su: not zero, but doesn't fix the key issue. Please be part of the solution here rather than just increasing pressure. I'm with you that this should have been fixed long ago but a day or two more will not change everything now. And rushing a bad fix now will have the bad fix end up everywhere and then the improved next iteration may not get enough attention to be applied everywhere later too. So the first shot is important here. Thanks for your understanding.

hartwork avatar Mar 17 '23 16:03 hartwork

You're right that bikeshedding wasn't the right word. What I meant is that this is going to be a long and difficult discussion and I'd like the manpage to mention the issue in the meantime.

e00E avatar Mar 17 '23 23:03 e00E

This is not an OpenDoas bug. TIOCSTI is a kernel problem.

jdebp avatar May 27 '23 02:05 jdebp

@jdebp no it's not. A process with different privileges should not be granted permissions to the controlling terminal in the first place.

hartwork avatar May 27 '23 10:05 hartwork

Thanks for writing and linking this page, jdebp. I don't know whether this is technically a kernel or an opendoas problem. From my linux user perspective I experienced this problem with doas and sudo and these tools have some ability to mitigate it. Personally, I would like to disable TIOCSTI gobally but this didn't used to be possible. Your page says at the end that this is possible now. Although I'm not sure when that comes to my distribution and what exactly I need to do.

The specific already possible thing I wanted to have happen with this bug report, is to document the problem in the opendoas man page. Regardless of whether it is an opendoas bug, users should be aware of it. If the problem can now be fixed with a sysctl then that's great and should also be in the man page.

e00E avatar May 27 '23 11:05 e00E

Note, since 83efeeeb3d04b22aaed1df99bc70a48fe9d22c4d Linux allows users to disable TIOCSTI. This can be done either at build-time via LEGACY_TIOCSTI, either at runtime via the dev.tty.legacy_tiocsti sysctl.

emersion avatar Nov 06 '23 13:11 emersion

@emersion it's also TIOCLINUX. Did the patch allowing to disable also get merged by now?

hartwork avatar Nov 06 '23 14:11 hartwork

Yes: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8d1b43f6a6df7bcea20982ad376a000d90906b42

emersion avatar Nov 06 '23 14:11 emersion

@emersion okay nice, merged to master but not in any release yet if I'm reading https://github.com/torvalds/linux/commit/8d1b43f6a6df7bcea20982ad376a000d90906b42 right (unlike the TIOCSTI one).

hartwork avatar Nov 06 '23 14:11 hartwork

FYI the TIOCLINUX patch is in the Linux 6.7 stable release now.

Thank you for documenting the issue so well here!

gnoack avatar Jan 30 '24 08:01 gnoack