klange / toaruos

A completely-from-scratch hobby operating system: bootloader, kernel, drivers, C library, and userspace including a composited graphical UI, dynamic linker, syntax-highlighting text editor, network stack, etc.
https://toaruos.org/
University of Illinois/NCSA Open Source License
6.03k stars 475 forks source link

Vulnerability in signals #270

Closed mxlgv closed 1 year ago

mxlgv commented 1 year ago

Hello! Found out that there is no check for a negative signal in the sigaction() function and some others. This allowed me to run a shell as root. Fix please! I hope you don't get offended that I do this. In any case, this is for the benefit of me and you!

klange commented 1 year ago

Classic case of mistakenly thinking this was an unsigned value, leading to a buffer underflow. Good catch! Simple fix.

mxlgv commented 1 year ago

@klange It looks like you haven't fixed it yet:https://github.com/klange/toaruos/blob/25fc89867116107018239d75738234d5ce67aa06/kernel/sys/signal.c#L214

klange commented 1 year ago

That's part of a different mechanism, but you might be right - looks like that still leaves an issue for some places sending signals with signed inputs. I'll take another look - maybe it would be better to make more of these just use an unsigned value!

klange commented 1 year ago

I was able to nicely cause a crash with a kernel page fault just with os.kill(..., -29353296) in Kuroko.

I put out another change in 441f853bc34a6d2e110fdfd6d4b48f82cb6924f7 that should hopefully resolve things, but do let me know if you spot anything else that looks suspicious. While I say that ToaruOS isn't intended to be secure, I still do appreciate reports of security issues as bugs, and low-hanging fruit like this is always worth addressing.