-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
example should guard against null string pointer #44
Comments
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. |
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! |
Yup, as deian mentioned, we are currently redesigning APIs to encourage C++ style usage patterns (which clarify things like ownership etc.)
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 |
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()
inmylib.c
to invokecb(NULL)
, runninghello
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 thecb()
callback, in the verifier thathello_cb()
passes tocopy_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.
The text was updated successfully, but these errors were encountered: