Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Beep security update

debian.org

Post reply on HN
Print view View original

81–90 of 103 posts

Re: Beep security update

#81

Post by avian »

jwilk wrote: post #43
nxtafan wrote: post #3

https://github.com/johnath/beep/issues/11#issuecomment-37838...

The Debian patch with diff highlighting: 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 f…

> 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.

Re: Beep security update

#82

Post by justincormack »

avian wrote: post #81
jwilk wrote: post #43

Earlier quoted context omitted.

The Debian patch with diff highlighting: 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 f…

> 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.

Not all the legacy ioctls are single device only.

Re: Beep security update

#83

Post by tejasmanohar »

Kamshak wrote: post #39
madez wrote: post #19

Earlier quoted context omitted.

> In particular, it is not safe to call exit, free, ioctl, putchar, or perror from a signal handler. Why is it accepted by the compiler, then?

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…

I don't think there's a purely technical reason a non-standard linter cannot exist, but it's probably a large investment to build such a tool (filtering false positives, etc.) and no one has made one sufficiently good and popular enough to be used for a simple one-file-utility.

Re: Beep security update

#84

Post by tejasmanohar »

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. :)
It also appears /dev/console is overridable, https://github.com/johnath/beep/blob/master/beep.c#L149.

Re: Beep security update

#85

Post by jwilk »

avian wrote: post #81
jwilk wrote: post #43

Earlier quoted context omitted.

The Debian patch with diff highlighting: 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 f…

> 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.

Yeah, you can't create new files that way; but opening an existing file can have side effects (think of tape devices or named pipes).

Re: Beep security update

#86

Post by 0x0 »

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 :)

Re: Beep security update

#87

Post by kelnos »

0x0 wrote: post #86

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 securit…

From https://packages.debian.org/source/stable/beep you can download the tarball of the original sources, as well as the debian patch on top of it.

Re: Beep security update

#88

Post by jwilk »

jfindley wrote: post #36

I'd love to hear the backstory. Who on _earth_ goes looking for vulnerabilities in beep?!

I wouldn't assume that someone was actively looking for vulnerabilities there.

Perhaps a user got curious how is this implemented, looked at the source, and was horrified with what they saw.

Re: Beep security update

#89

Post by tinus_hn »

madez wrote: post #19
garethrees wrote: post #15

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. [1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2...

> In particular, it is not safe to call exit, free, ioctl, putchar, or perror from a signal handler. Why is it accepted by the compiler, then?

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.

Re: Beep security update

#90

Post by [deleted] »

justincormack wrote: post #82
avian wrote: post #81

Earlier quoted context omitted.

> 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.

Not all the legacy ioctls are single device only.

[deleted]
Post reply on HN