Decoder may expose contents of uninitialized memory in the output

[Shnatsel]

libflate might expose contents of uninitialized memory in the output when given a crafted input. This may be a devastating vulnerability in some contexts, e.g. if used as deflate backend for a PNG decoder. Details and impact analysis for similar bugs in PNG decoders in C can be found here.

I am confident that a private function is vulnerable, but I am not sure if this vulnerability can be exploiter by supplying a malformed input; there could be some non-local checks that prevent it.

I shall relay further details on the issue to the maintainer privately by email.

[sile]

I have received the email. Thanks!

[Shnatsel]

This was fixed by #38. The bug is in the following code:

libflate/src/deflate/decode.rs

Lines 107 to 127 in 71d12d2

	symbol::Symbol::Share { length, distance } => {
	if self.buffer.len() < distance as usize {
	return Err(invalid_data_error!(
	"Too long backword reference: buffer.len={}, distance={}",
	self.buffer.len(),
	distance
	));
	}
	let old_len = self.buffer.len();
	self.buffer.reserve(length as usize);
	unsafe {
	self.buffer.set_len(old_len + length as usize);
	let start = old_len - distance as usize;
	let ptr = self.buffer.as_mut_ptr();
	util::ptr_copy(
	ptr.add(start),
	ptr.add(old_len),
	length as usize,
	length > distance,
	);
	}

This code never checks that distance is not 0. If it is and length is > 0, the following happens:

• self.buf gets unsafely grown to accommodate additional length elements and now contains uninitialized memory
• util::ptr_copy gets called with parameters (old_len, old_len, length, true) which causes it to walk over the uninitialized memory region, reading every byte from it and immediately putting it back in its place
• execution of the function ends, leaving self.buf with contents of uninitialized memory exposed in it

This may be a devastating vulnerability in some contexts, e.g. if used as deflate backend for a PNG decoder. Details and impact analysis for similar bugs in PNG decoders in C can be found here.

I am confident that this private function is vulnerable, but I am not sure if this vulnerability can be exploited by supplying a malformed input - there might be some non-local checks that prevent it.

Please fix the bug and check if it's possible to trigger it through a crafted input file, either via setting distance to 0 or via overflowing distance. If it is indeed possible, please file a security advisory at https://github.com/RustSec/advisory-db

[sile]

there might be some non-local checks that prevent it.

Yes.
The value of distance is detemined by Decoder::decode_distance method (https://github.com/sile/libflate/blob/master/src/deflate/symbol.rs#L224).
The method refers to DISTANCE_TABLE constant to look up the values of base (range: 1..=24_557) and extra_bits (range: 0..=13) variables, and returns base + ${read extra_bits from bits-stream) as the distance value.
So the minimum value of distance is 1, and the maximum value is 24_577 + ((1<<13) - 1) = 32_768 < std::u16::MAX.

[Shnatsel]

Thanks! I'm closing this issue then.

[sile]

Thank you very much for your contribution!