Tonic's `encode_item` function is not exception safe, leading to use of uninitialized memory

[Evian-Zhang]

Bug Report

Version

0.14.6

Platform

Linux

Crates

tonic

Description

Hello, we are security researchers targeting Rust security. By running our tool in this repository, we found a soundness issue. This report is written by 100% human. We promise that all you read will never be generated by LLM.

The problem happens here:

grpc-rust/tonic/src/codec/encode.rs

Lines 133 to 179 in b68df9f

	fn encode_item<T>(
	encoder: &mut T,
	buf: &mut BytesMut,
	uncompression_buf: &mut BytesMut,
	compression_encoding: Option<CompressionEncoding>,
	max_message_size: Option<usize>,
	buffer_settings: BufferSettings,
	item: T::Item,
	) -> Result<(), Status>
	where
	T: Encoder<Error = Status>,
	{
	let offset = buf.len();
	buf.reserve(HEADER_SIZE);
	unsafe {
	buf.advance_mut(HEADER_SIZE);
	}
	if let Some(encoding) = compression_encoding {
	uncompression_buf.clear();
	encoder
	.encode(item, &mut EncodeBuf::new(uncompression_buf))
	.map_err(|err| Status::internal(format!("Error encoding: {err}")))?;
	let uncompressed_len = uncompression_buf.len();
	compress(
	CompressionSettings {
	encoding,
	buffer_growth_interval: buffer_settings.buffer_size,
	},
	uncompression_buf,
	buf,
	uncompressed_len,
	)
	.map_err(|err| Status::internal(format!("Error compressing: {err}")))?;
	} else {
	encoder
	.encode(item, &mut EncodeBuf::new(buf))
	.map_err(|err| Status::internal(format!("Error encoding: {err}")))?;
	}
	// now that we know length, we can write the header
	finish_encoding(compression_encoding, max_message_size, &mut buf[offset..])
	}

In line 149, buf.advance_mut is called to increase the length for header, and in line 178, finish_encoding is called to fill header bytes into the increased slots. However, in line 156 and line 173, Encoder::encode method is called, which is a user-provided trait method. If user deliberately call panic! inside encode function, finish_encoding will never be called, leading to buf exposing uninitialized variable.

To fix it, I think adding a drop guard to restore the length when unwinding could help.

[Evian-Zhang]

I suspect the link posted above is malicious, considering the repo has no code, and the user was registered just several hours ago. I don't know this guy. Please be cautious if you click that link.

[dfawley]

Thanks @Evian-Zhang - I deleted the message and reported the user. The repo they posted the link from was already removed.