Lack of bound check while loading elf32 segments

EtchedPixels / FUZIX

FuzixOS: Because Small Is Beautiful

Other

2.16k stars 271 forks source link

Lack of bound check while loading elf32 segments #887

Closed chatrasc closed 2 years ago

chatrasc commented 2 years ago

I noticed a lack of bound check while loading elf32 segments.

https://github.com/EtchedPixels/FUZIX/blob/master/Kernel/syscall_execelf32.c#L208

I currently run FUZIX on RP2040 platform which does not have any memory protection. But I supposed it is not an expected behavior as it would allow an arbitrary write with kernel privileges on platforms supporting MPU or MMU.

If you dont mind I will propose a pull request for this issue.

EtchedPixels commented 2 years ago

The segments are loaded with sysio false, which means they are loaded as if a user application did read(). That should mean any invalid or out of range stuff is errored in readi() ?

chatrasc commented 2 years ago

It seems valaddr() is called before readi() if an application calls read(). We may need to do it as well for the elf segments ?

Do you want me to share my test file?

EtchedPixels commented 2 years ago

And readi uses uputblk for partials but if you have full blocks doesn't check. So yes it needs a valaddr(), and even better if all the callers are audited to use valaddr for sysio = false uputblk can use _uput for speed.

chatrasc commented 2 years ago

Thanks to your previous comments, I was able to investigate a bit more. valaddr() is properly called by uput() when loading segments. I think my issue is platform specific (platform-rpipico).

While loading crafted ELF I was questionning two things:

I was able to write onto udata (I supposed it is not expected as it has euid, ptab pointer, etc.). That is probably because udata is set at progbase. https://github.com/EtchedPixels/FUZIX/blob/master/Kernel/platform-rpipico/config.h#L43
I was also able to write onto ptab. That is probably because their is no check for base being greater than top in valaddr() of rpipico. https://github.com/EtchedPixels/FUZIX/blob/master/Kernel/platform-rpipico/misc.c#L32

Again, memory protection is not supported on this platform so that's not a big deal.

EtchedPixels commented 2 years ago

Just my own notes for when I go fixing this properly. The steps needed are

audit all calls to readi/writei that do I/O to user space and ensure they use valaddr at source
switch uput/uget checks in readi/writei paths to _uput and _uget for speed

EtchedPixels commented 2 years ago

Should now be dealt with properly