[libvirt] [PATCH 0/2] libvirt-rust: Fixing bugs in Stream::{send, recv}

[Linus Färnstrand]

Hi,

Fixing memory soundness bugs in Stream::recv, and other bugs in both
Stream::recv and Stream::send.

The method Stream::recv takes a size argument that it passes on as the nbytes
argument to virStreamRecv. The problem is that the corresponding data pointer
points to a local stack allocated array with a fixed size of 2048. So calling
Stream::recv with a size larger than 2048 can cause virStreamRecv to write
past the given buffer!

The library calls CStr::from_ptr(data) on the locally allocated buffer after
virStreamRecv returns. CStr::from_ptr will scan the memory from the given
pointer for the terminating null byte. There is no guarantee this buffer will
properly end with a null byte. The buffer is initialized with only nulls.
But since virStreamRecv can overwrite the entire buffer (and beyond!), there
is no guarantee there will be a null byte in the buffer at this point. As a
result, CStr::from_ptr can continue to read past the buffer. This can cause
either reading invalid memory, or it can cause the method to return a string
containing data from the stack way past the data written by virStreamRecv.
Heartbleed style issue.

The code does not check for the -2 error code in the return value of
virStreamRecv, treating it as a success. It looks like it will make it just
return an empty string as result. But a bug nonetheless.

After parsing the buffer as a C string in Stream::recv, it is lossily converted
into a Rust UTF-8 string with CStr::to_string_lossy. It can cause the data to
be modified before it is returned to the caller.
U+FFFD REPLACEMENT CHARACTER will be inserted on invalid UTF-8 codepoints.

The FFI bindings for both virStreamRecv and virStreamSend are wrong. They
specify the nbytes argument as c_int instead of size_t as the C library does.

I don't have access to SMTP on my dev machine. Hope ProtonMail does not
reformat this.

Have a great day
Linus Färnstrand

Linus Färnstrand (2):
  Fix bugs in Stream::recv
  Fix Stream::send

 src/stream.rs | 42 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

--
2.21.0