Could the :string type accepts Ruby Strings with null bytes, or an alternative type accept them?

[eregon]

From oracle/truffleruby#3293 (comment)
Related to #1080

:pointer is not great because it forces the passed Ruby String to use native memory, or copies before & after the native call.
:string avoids this but currently forbids null bytes:

irb(main):001> module MyLib; extend FFI::Library; ffi_lib 'c'; attach_function :strtok, [ :pointer, :string ], :pointer; end
=> #<FFI::Function address=0x00007ff96ecc5680>
irb(main):002> s = "hello"
=> "hello"
irb(main):005> MyLib.strtok(s, "a\0b")
(irb):5:in `strtok': string contains null byte (ArgumentError)

https://github.com/ffi/ffi/wiki/Types#types mentions :string as

C-style (NULL-terminated) character string.

But if there is a \0 in the middle it's still NULL-terminated from C's point of view.
Also C code can take a const char *string, size_t length for instance and deal with \0 bytes in the middle.

So I think we should remove that check because it's not really necessary for safety and it prevents using :string where it matters to avoid extra copying.

The only advantage of the check I see is it could catch an unintentional String with a \0 in it passed to FFI, and this could matter if the C code uses strlen() or similar on it. But having it at the cost of not allowing valid usages seems not worth.

[enebo]

@eregon unintentional string data with \0 is one of the largest sources of CVEs in the world. Also probably the largest cause of CVEs for MRI. From FFI? I am not sure as I don't remember one but I would be extremely hesitant about assuming people are processing char * data correctly.

[eregon]

Mmh, right. So just allowing it seems not a good solution.
And such cases do seem to be considered to be CVEs, example even though in many cases I'd think it'd be fairly harmless.
I think the largest cause of CVEs in MRI is ReDoS, it's been the majority of recent CVEs.

Maybe we should have a way to specify if :pointer is in/out/inout, that would avoid extra copying when passing a Ruby String as a buffer.
Maybe with :pointer_in, :pointer_out types, and existing :pointer would be inout for compatibility?

[eregon]

Existing workarounds for this are to manually use a MemoryPointer.
For in-only, use MemoryPointer.from_string(str) (or do it manually to use the block form) and pass that.
For out-only, pass a MemoryPointer.new(:char, size, clear=false) and then read_string/read_bytes.
So it's not too bad, but I think the existing types would make it easier to optimize these cases.
It can matter a lot for performance, e.g. in ruby/prism#2402 (comment) it's >10x faster without the extra copying.

[enebo]

@eregon I am only against changing string.

If there is some way of making the memory pointer use cases easier then I think it is a good idea. I am not the gatekeeper of FFI APIs though 😄. I was just concerned about security issues of changing string.

If you come up with something more specific then open up a new PR and the folks who tend to triage FFI issues more can chime in. I will close this and when/if you open something we can reference this issue for background.