The author of beep needs to read the POSIX specification on async-signal-safety [1]. In particular, it is not safe to call exit, free, ioctl, putchar, or perror from a signal handler.
This is a good question and doesn't deserve to be downvoted.
I think the reason must be that the problem lies in the intersection of three areas of development (the C programming language, the C standard library, and the POSIX operating system definition) and so requires coordination to solve.
Think about how you would implement warnings for failures of async-signal-safety. One approach would go like this:
1. In compiler front-ends, introduce a new attribute that can be attached to function declarations to indicate that they are async-signal-safe, for example __attribute__((async_signal_safe)), and a new attribute that can be attached to function parameters and struct members to indicate that the passed value must be async-signal-safe, for example __attribute__((require_async_signal_safe)).
2. In C library headers, apply the async_signal_safe attribute to all the async-signal-safe function declarations, and apply the require_async_signal_safe attribute to the parameter to the signal function and to the sa_handler member of struct sigaction.
3. In compilers, propagate the async_signal_safe attribute, so that any function that only calls async_signal_safe functions is also marked with the attribute.
4. In compilers, check that when a function is passed to a parameter or assigned to a member with the require_async_signal_safe attribute, the function is marked with the async_signal_safe attribute, and issue a warning if not.
This doesn't solve the whole problem (sometimes the compiler will not be able to know which function is going to be passed as the parameter to signal, because this is determined at runtime), and it might have false positives in obscure situations (you might in theory use a struct sigaction for some other purpose and never pass it to sigaction) but it would catch many cases, including the one in beep.
But notice the amount of coordination required. It would require input from several people with different areas of expertise.
I'll be quicker to blame the signal API than the programming language on that front. Dealing with unix signals correctly and robustly is far from trivial and rife with footguns. For instance I believe that Rust still doesn't have a good general purpose solution for handling signals that doesn't involve the libc and unsafe code.
Signal is basically the crappiest form of IPC available on a modern operating system short of emulating a mouse and keyboard and typing into the other application's terminal window.
Because the compiler doesn't and cannot know the semantics of signal(2)/sigaction(2). There's what POSIX says, which a compiler could enforce using heuristics, and there's what your C library says. It's perfectly plausible to make a number of nominally not-async-signal-safe functions from the standard actually async-signal-safe in a particular C library. Granted, if you were to take advantage of such an extension, your code would not be portable.
(EDIT: The compiler doesn't even know that you're using POSIX. It can figure it out contextually. It doesn't know which C library you'll be linking with though. Bottom line: we need some C standards extensions, or failing that, GCC/Clang C extensions in order to best handle this, though there are some heuristics a compiler could implement even without those, at some mild risk.)
(E.g., suppose there was a thread-local counter of signal handlers on the stack... then stdio functions might be able to handle async-signal reentrance. It's not too farfetched, though it's obviously a lot easier to not bother at all and just leave the set of async-signal-safe functions being just the set of system calls that have sufficiently thin stubs in the C library.)
Now, the C standard could have additional keywords, much like, say, 'volatile' and friends, to describe async-signal-safety, reentrance, and other characteristics of functions. And if the C library and your programs used these then the compiler absolutely could warn or refuse to compile your program when you break the standard's rules.
Incidentally, Unix/POSIX signals are horrible. The only sane and portable way to handle them in programs that do I/O is to have an event loop and a "self-pipe" that the signal handler can write into so that the event loop can pick up and handle the signal as a normal async event in a context where there are no constraints on calling async-signal-unsafe functions. This is what I always do in my programs. I strongly recommend it. This means you don't need ppoll(2), pselect(2), signalfd(2), epoll_pwait(2), and so on -- you don't because you're always turning signals into normal I/O events, so you don't need to worry about signal blocking, and you don't need to use less widely available functions.
It can't be done in general. A signal handler is just a function that at runtime is connected to a signal. With some work many or most unsafe handlers in the wild can be detected, but so can many other problems, I guess.
This is a really good point actually, aren't there static analysis tools that can figure out things like this? I know in JS we have linters to stop you from doing things that are most likely not very smart.
I find this very useful, a lot more useful than to say "If you don't even read the manpages there is no help in sight".
Maybe the compiler should not do it but perhaps you could run a linter against packages that are shipped with an os to find issues like this easier.
A signal handler is not special, it's just another function. So the compiler can't really apply the rules. Also this is a very limited subsection of all the many things you are not supposed to do in a signal handler.
It’s undefined behavior, which means the compiler is free to accept it and do something sensible, or failing subtle ways, or make demons fly out of your nose.
By default beep is not installed with the suid bit set, because that would just be zany. On the other hand, if you do make it suid root, all your problems with beep bailing on ioctl calls will magically vanish, which is pleasant, and the only reason not to is that any suid program is a potential security hole. Conveniently, beep is very short, so auditing it is pretty straightforward.
patch calls /bin/ed. /bin/ed has a ! command that feeds stuff to /bin/sh. Feeding unreviewed patches to the patch command is actually arbitrary command execution.
This is particularly awesome if patch is being used by other programs (i.e. a CI pipeline or other contexts).
Not sure if you're aware of that but it's a joke page made as a parody of the recent "branded vulnerability" craze. I would definitely advise against running random scripts from joke pages, especially through sudo. In that regard notice the "TODO" line from the script:
I don't see the need to qualify "curl | sudo bash" with it being a two line script for it to be problematical.
"curl | bash" or "curl | sudo bash" is problematical no matter how large or small the script.
Yes, I know some people say that it is no worse than downloading to a file and then running the file without reading the script, which is what almost everyone does anyway.
These people are wrong. Downloading to a file and running it from there is always better because if you notice something wonky sometime after running the script, it is easier to prove the script caused it if you saved a copy before running it.
If you "curl | bash" it and then you notice something bad has happened and you want to look at the script, you have to curl it again. But then how do you know that second curl gives the same script as the one you just executed? If I were distributing a malicious script, I would set up my server to serve a non-malicious script most of the time and just occasionally substitute my malware script, and it would only distribute the malware once for each IP address.
Either do "curl > file; bash < file" of "curl | tee file | bash" rather than "curl | bash" if you aren't interesting in examining the script before running it.
If I had to guess, I'd say they were doing it because it is often installed with suid root.
Edit: also, there's a challenge about this in the program's README :-)
"Decide for yourself, of course, but it looks safe to me - there's only one
buffer and fgets doesn't let it overflow, there's only one file opening, and while there is a potential race condition there, it's with /dev/console. If someone can exploit this race by replacing /dev/console, you've got bigger problems. :)"
It should be pointed out that the README is wrong about the race condition – it is not limited to /dev/console, you can use the -e option to point it to a different output device.
3) Makes sure that the device is opened only once.
I still don't understand what exactly the bug is supposed to be. I also don't understand what is the purpose of 3). With this fix applied, it's still possible for a user to open arbitrary file for writing with root privileges, which is bad.
My speculation on the race condition fixed in the patch:
The while loop in `main` calls `play_beep` multiple times. Each call to `play_beep` opens the `--device` and sets the global `console_fd`, and then sets the global `console_type` based on the `ioctl(EVIOCGSND)` error, before calling `do_beep`.
This normally prevents the user from writing to arbitrary files with `--device`, because without the `ioctl(EVIOCGSND)` succeeding, `do_beep` with `BEEP_TYPE_CONSOLE` only does a (harmless?) `ioctl(KIOCSOUND)`, not a `write` with the `struct input_event`. However, the signal handler calls `do_beep` directly using the globals set by `play_beep`...
So I image that with something along the lines of `beep --device=./symlink-to-tty ... --new ...`, you can rewrite the symlink to point to an arbitrary file during the first `play_beep`, and then race the open/ioctl in the second `play_beep` with the signal handler such that `do_beep` gets called with `console_fd` pointing to your arbitrary file, and with `console_type` still set to `BEEP_TYPE_EVDEV`, resulting in a `write` to your arbitrary file.
Exploiting that for privesc would require control over the `struct input_event` for the `write`... `handle_signal` calls `do_beep` with a fixed `freq` of 0, so all of the initialized fields are set to fixed values... However, there's an unitialized `struct timeval` at the beginning of the `struct input_event`, and it's allocated on the stack...
Seems like a curious security vulnerability, I'll assume the debian security team must have a working PoC in order to actually call it out as a local privesc vulnerability... I'd love to see the actual PoC eventually :)
> it's still possible for a user to open arbitrary file for writing with root privileges, which is bad.
It appears that way at first, but: O_WRONLY mode will not create a new file, so you can't make files as root. If you open an existing file, it will fail later on the console device-specific ioctl. If that's the catch here, it does not seem trivial to exploit.
anyway? Or if you're trying to do something fancier there is snd-pcsp which lets use use the piezo as a normal ALSA device (quality may vary). Not that many modern machines even include such a useful device.
I have long wished servers had functioning speakers.
And that virtio-beep existed.
I wonder what would happen if I pushed to get the support into KVM and the kernel. Best case scenario, the videos accompanying the too-bemused-to-really-be-angry bug reports would be legendary.
Does GPL say anything about distributing source via git repositories? I mean, it talks about "preferred form of the work for making modifications". But when I click through to the Debian PTS page for Beep at https://tracker.debian.org/pkg/beep it shows a git link to http://git.deb.at/w/pkg/beep.git which was last modified 23 months ago, so obviously does not contain this security fix. How is the GPL upheld if security fixes aren't available together with the main git repository?
I don't mean to cause drama, because I think the Debian security team does an awesome job of keeping an eye on patching everything in the distribution. But I'm really curious about how one would go about with further development on a package like this when the git commits aren't available? There's probably a source tarball for the .debs in the apt repos (hard to tell from a mobile web browser) but is that good enough if it doesn't contain git commits?
Also I really just wanted to read this security patch source diff on the go :)
(N.B.: I'm on the Debian ftpmaster team, which reviews all new software in Debian to ensure it matches our policies and commitments)
> Does GPL say anything about distributing source via git repositories?
Nope.
> I mean, it talks about "preferred form of the work for making modifications".
This is generally interpreted as a fancy way of saying "source code" — e.g. a minified JavaScript program or something that could be decompiled isn't the "preferred form… for making modifications".
Source for all Debian packages can be retrieved via `apt-get source PACKAGE`, and via sources.debian.org[3].
Historically, the only hard requirement was that all patches be made available as a `.diff.gz` applied against the upstream, although for many years packages have since standardised on using `quilt`[2]. Some maintainers may also use git (and wrappers around it) to make things easier to manage, but quilt is sufficiently straightforward to work with that it's acceptable for most purposes.
The VCS page on the BTS points to the repository for the package. It contains an unreleased version (1.3.3-5), the security update only is a patch on the previous version released in Debian.
The last upstream release was 1.3 (2010), available on GitHub[1]. This is a Debian-specific patch against beep. We'd like all patches to make it upstream, and ensure maximum clarity, but we have limited time, and our first priority is the users of Debian. Sometimes, upstreams lose time, or there are coordination issues that prevent it. (not saying that's the case here)
I'm still curious about the general case of GPL and Git. If I were to hack on some GPL-licensed software, my "preferred form of the work for making modifications" would hands-down be a .git repository, because all the individual git commits and commitmsgs are important (meta-)information, almost as important as the source code files themselves. It also makes collaborating on improvements much easier when you can work on git commit objects, with their parent(s) commit hash references. Without those, determining where and if a .patch should be applied to a source code dump becomes much harder.
In other words, if there exists a .git repository for a given piece of software, and all one gets is a .tar.gz flat source dump snapshot, I feel like... something has been left out?
Is it worrisome that the Debian distribution comes with a package with this note in the README
Decide for yourself, of course, but it looks safe to me -
there's only one buffer and fgets doesn't let it overflow,
there's only one file opening, and while there is a
potential race condition there, it's with /dev/console.
If someone can exploit this race by replacing /dev/console,
you've got bigger problems. :)
FWIW, that comment was written before the option to control the path beep uses was added. Without the option to control the path, the potential race is only exploitable by somebody who can write to /dev, in which case that person is already root.
Readit News
[1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2...
Why is it accepted by the compiler, then?
I think the reason must be that the problem lies in the intersection of three areas of development (the C programming language, the C standard library, and the POSIX operating system definition) and so requires coordination to solve.
Think about how you would implement warnings for failures of async-signal-safety. One approach would go like this:
1. In compiler front-ends, introduce a new attribute that can be attached to function declarations to indicate that they are async-signal-safe, for example __attribute__((async_signal_safe)), and a new attribute that can be attached to function parameters and struct members to indicate that the passed value must be async-signal-safe, for example __attribute__((require_async_signal_safe)).
2. In C library headers, apply the async_signal_safe attribute to all the async-signal-safe function declarations, and apply the require_async_signal_safe attribute to the parameter to the signal function and to the sa_handler member of struct sigaction.
3. In compilers, propagate the async_signal_safe attribute, so that any function that only calls async_signal_safe functions is also marked with the attribute.
4. In compilers, check that when a function is passed to a parameter or assigned to a member with the require_async_signal_safe attribute, the function is marked with the async_signal_safe attribute, and issue a warning if not.
This doesn't solve the whole problem (sometimes the compiler will not be able to know which function is going to be passed as the parameter to signal, because this is determined at runtime), and it might have false positives in obscure situations (you might in theory use a struct sigaction for some other purpose and never pass it to sigaction) but it would catch many cases, including the one in beep.
But notice the amount of coordination required. It would require input from several people with different areas of expertise.
Signal is basically the crappiest form of IPC available on a modern operating system short of emulating a mouse and keyboard and typing into the other application's terminal window.
(EDIT: The compiler doesn't even know that you're using POSIX. It can figure it out contextually. It doesn't know which C library you'll be linking with though. Bottom line: we need some C standards extensions, or failing that, GCC/Clang C extensions in order to best handle this, though there are some heuristics a compiler could implement even without those, at some mild risk.)
(E.g., suppose there was a thread-local counter of signal handlers on the stack... then stdio functions might be able to handle async-signal reentrance. It's not too farfetched, though it's obviously a lot easier to not bother at all and just leave the set of async-signal-safe functions being just the set of system calls that have sufficiently thin stubs in the C library.)
Now, the C standard could have additional keywords, much like, say, 'volatile' and friends, to describe async-signal-safety, reentrance, and other characteristics of functions. And if the C library and your programs used these then the compiler absolutely could warn or refuse to compile your program when you break the standard's rules.
Incidentally, Unix/POSIX signals are horrible. The only sane and portable way to handle them in programs that do I/O is to have an event loop and a "self-pipe" that the signal handler can write into so that the event loop can pick up and handle the signal as a normal async event in a context where there are no constraints on calling async-signal-unsafe functions. This is what I always do in my programs. I strongly recommend it. This means you don't need ppoll(2), pselect(2), signalfd(2), epoll_pwait(2), and so on -- you don't because you're always turning signals into normal I/O events, so you don't need to worry about signal blocking, and you don't need to use less widely available functions.
I find this very useful, a lot more useful than to say "If you don't even read the manpages there is no help in sight".
Maybe the compiler should not do it but perhaps you could run a linter against packages that are shipped with an os to find issues like this easier.
By default beep is not installed with the suid bit set, because that would just be zany. On the other hand, if you do make it suid root, all your problems with beep bailing on ioctl calls will magically vanish, which is pleasant, and the only reason not to is that any suid program is a potential security hole. Conveniently, beep is very short, so auditing it is pretty straightforward.
https://github.com/johnath/beep
Heh
http://git.savannah.gnu.org/cgit/patch.git/tree/src/pch.c#n2...
patch calls /bin/ed. /bin/ed has a ! command that feeds stuff to /bin/sh. Feeding unreviewed patches to the patch command is actually arbitrary command execution.
This is particularly awesome if patch is being used by other programs (i.e. a CI pipeline or other contexts).
* curl | sudo bash for two lines of script
* said script just checks if you have beep installed, not if you're vulnerable
"curl | bash" or "curl | sudo bash" is problematical no matter how large or small the script.
Yes, I know some people say that it is no worse than downloading to a file and then running the file without reading the script, which is what almost everyone does anyway.
These people are wrong. Downloading to a file and running it from there is always better because if you notice something wonky sometime after running the script, it is easier to prove the script caused it if you saved a copy before running it.
If you "curl | bash" it and then you notice something bad has happened and you want to look at the script, you have to curl it again. But then how do you know that second curl gives the same script as the one you just executed? If I were distributing a malicious script, I would set up my server to serve a non-malicious script most of the time and just occasionally substitute my malware script, and it would only distribute the malware once for each IP address.
Either do "curl > file; bash < file" of "curl | tee file | bash" rather than "curl | bash" if you aren't interesting in examining the script before running it.
> No.
> How do I uninstall Linux?
> Please follow instructions.
Shirley it's not meant as a serious resource.
Edit: also, there's a challenge about this in the program's README :-)
"Decide for yourself, of course, but it looks safe to me - there's only one buffer and fgets doesn't let it overflow, there's only one file opening, and while there is a potential race condition there, it's with /dev/console. If someone can exploit this race by replacing /dev/console, you've got bigger problems. :)"
Perhaps a user got curious how is this implemented, looked at the source, and was horrified with what they saw.
https://gist.github.com/jwilk/561ae35894756aae1e31503d0c52db...
AFAICS, it does this:
1) Fixes use of uninitialized memory.
2) Fixes double-free in the signal handler.
3) Makes sure that the device is opened only once.
I still don't understand what exactly the bug is supposed to be. I also don't understand what is the purpose of 3). With this fix applied, it's still possible for a user to open arbitrary file for writing with root privileges, which is bad.
The while loop in `main` calls `play_beep` multiple times. Each call to `play_beep` opens the `--device` and sets the global `console_fd`, and then sets the global `console_type` based on the `ioctl(EVIOCGSND)` error, before calling `do_beep`.
This normally prevents the user from writing to arbitrary files with `--device`, because without the `ioctl(EVIOCGSND)` succeeding, `do_beep` with `BEEP_TYPE_CONSOLE` only does a (harmless?) `ioctl(KIOCSOUND)`, not a `write` with the `struct input_event`. However, the signal handler calls `do_beep` directly using the globals set by `play_beep`...
So I image that with something along the lines of `beep --device=./symlink-to-tty ... --new ...`, you can rewrite the symlink to point to an arbitrary file during the first `play_beep`, and then race the open/ioctl in the second `play_beep` with the signal handler such that `do_beep` gets called with `console_fd` pointing to your arbitrary file, and with `console_type` still set to `BEEP_TYPE_EVDEV`, resulting in a `write` to your arbitrary file.
Exploiting that for privesc would require control over the `struct input_event` for the `write`... `handle_signal` calls `do_beep` with a fixed `freq` of 0, so all of the initialized fields are set to fixed values... However, there's an unitialized `struct timeval` at the beginning of the `struct input_event`, and it's allocated on the stack...
Seems like a curious security vulnerability, I'll assume the debian security team must have a working PoC in order to actually call it out as a local privesc vulnerability... I'd love to see the actual PoC eventually :)
It appears that way at first, but: O_WRONLY mode will not create a new file, so you can't make files as root. If you open an existing file, it will fail later on the console device-specific ioctl. If that's the catch here, it does not seem trivial to exploit.
And that virtio-beep existed.
I wonder what would happen if I pushed to get the support into KVM and the kernel. Best case scenario, the videos accompanying the too-bemused-to-really-be-angry bug reports would be legendary.
The portable way to print ASCII BEL is:
I don't mean to cause drama, because I think the Debian security team does an awesome job of keeping an eye on patching everything in the distribution. But I'm really curious about how one would go about with further development on a package like this when the git commits aren't available? There's probably a source tarball for the .debs in the apt repos (hard to tell from a mobile web browser) but is that good enough if it doesn't contain git commits?
Also I really just wanted to read this security patch source diff on the go :)
> Does GPL say anything about distributing source via git repositories?
Nope.
> I mean, it talks about "preferred form of the work for making modifications".
This is generally interpreted as a fancy way of saying "source code" — e.g. a minified JavaScript program or something that could be decompiled isn't the "preferred form… for making modifications".
Source for all Debian packages can be retrieved via `apt-get source PACKAGE`, and via sources.debian.org[3].
Historically, the only hard requirement was that all patches be made available as a `.diff.gz` applied against the upstream, although for many years packages have since standardised on using `quilt`[2]. Some maintainers may also use git (and wrappers around it) to make things easier to manage, but quilt is sufficiently straightforward to work with that it's acceptable for most purposes.
The VCS page on the BTS points to the repository for the package. It contains an unreleased version (1.3.3-5), the security update only is a patch on the previous version released in Debian.
The last upstream release was 1.3 (2010), available on GitHub[1]. This is a Debian-specific patch against beep. We'd like all patches to make it upstream, and ensure maximum clarity, but we have limited time, and our first priority is the users of Debian. Sometimes, upstreams lose time, or there are coordination issues that prevent it. (not saying that's the case here)
[1]: http://github.com/johnath/beep/ [2]: https://wiki.debian.org/UsingQuilt [3]: https://sources.debian.org/src/beep/unstable/
I'm still curious about the general case of GPL and Git. If I were to hack on some GPL-licensed software, my "preferred form of the work for making modifications" would hands-down be a .git repository, because all the individual git commits and commitmsgs are important (meta-)information, almost as important as the source code files themselves. It also makes collaborating on improvements much easier when you can work on git commit objects, with their parent(s) commit hash references. Without those, determining where and if a .patch should be applied to a source code dump becomes much harder.
In other words, if there exists a .git repository for a given piece of software, and all one gets is a .tar.gz flat source dump snapshot, I feel like... something has been left out?
But sources.d.o seem to lack security updates (or at least the security update for this package is missing). :-\
True but same idea applies. If your end device is maliciously replaceable you've got bigger problems.