The current implementation of rb_get_args to get ruby strings is unsafe. When the format string uses z, it returns a char* using RSTRING_PTR, which isn't even guaranteed to be null terminated. It's sort of a miracle that we haven't seen more crashes related to this (maybe plain ASCII strings are safe, but we sure as heck shouldn't rely on it).
This means only using s is safe, but its implementation actually has an error (not incrementing the pointer at all (binding-mri.cpp#169)) and is not used anywhere in the repo (only the objectLoad function uses it and that function isn't used anywhere). We need to fix this and either port all usages of z to s, or make z somehow return a null terminated string while not leaking memory (idk how to do that).
Speak2Erase
commented
2 years ago
The current implementation of
rb_get_argsto get ruby strings is unsafe. When the format string usesz, it returns achar*usingRSTRING_PTR, which isn't even guaranteed to be null terminated. It's sort of a miracle that we haven't seen more crashes related to this (maybe plain ASCII strings are safe, but we sure as heck shouldn't rely on it).This means only using
sis safe, but its implementation actually has an error (not incrementing the pointer at all (binding-mri.cpp#169)) and is not used anywhere in the repo (only theobjectLoadfunction uses it and that function isn't used anywhere). We need to fix this and either port all usages ofztos, or makezsomehow return a null terminated string while not leaking memory (idk how to do that).Thanks to @CryroFox for discovering this issue.