PLSysSec / rlbox

RLBox sandboxing framework
https://rlbox.dev
MIT License
285 stars 21 forks source link

example should guard against null string pointer #44

Open zeldovich opened 2 years ago

zeldovich commented 2 years ago

I was trying out the "hello world" example in rlbox, and wanted to confirm that it guards against the library returning garbage. When I changed call_cb() in mylib.c to invoke cb(NULL), running hello crashed with a segfault.

As far as I can tell, the example doesn't seem to properly guard against the library function call_cb passing a null string to the cb() callback, in the verifier that hello_cb() passes to copy_and_verify_string.

It would be good if the example showed what kinds of library mis-behavior the validators guard against (maybe I'm confused about NULL being a misbehavior that rlbox is guarding against?), and/or guard against the library returning a null string pointer.

deian commented 2 years ago

Yep this was a bad validator (on my part) that didn't check for the nullptr case.

We're working on docs + tutorials (with real libraries) that will hopefully illustrate what you're thinking about.

deian commented 2 years ago

We're (really @shravanrn) also redesigning some of the APIs (e.g., to avoid the C-like API and have convenient APIs like these) so if you have suggestions, happy to hear them!

shravanrn commented 2 years ago

Yup, as deian mentioned, we are currently redesigning APIs to encourage C++ style usage patterns (which clarify things like ownership etc.)

maybe I'm confused about NULL being a misbehavior that rlbox is guarding against?

This particular case is interesting in that passing a NULL string can be allowed behavior for some programs. So, we need to be careful with automatic RLBox checking here.

In the API redesign, it would be good to pay closer attention to NULLs. One solution could be to explicitly indicate in APIs whether NULL is ok or not. For example, we want to add a version of malloc_in_sandbox, say malloc_infallible_in_sandbox (still bikeshedding the name), which does NULL checking on the return. This could allow us to stop some denial of service style attacks (due to null pointer access) in addition to memory safety errors.