Devolutions/sspi-rs

Integer overflows in ber sizeof_ functions

Closed this issue · 2 comments

ekse commented

The sizeof functions in ber.rs (sizeof_sequence, sizeof_application_tag, sizeof_sequence_tag, etc.) can overflow.

For example, TsRequest::from_buffer can panic on large ts_request_len values.

let ts_request_len = ber::read_sequence_tag(&mut stream)
            .map_err(|e| io::Error::new(io::ErrorKind::UnexpectedEof, e))?;
if buffer.len() < ber::sizeof_sequence(ts_request_len) as usize {

We should either handle the overflow in the sizeof_ methods, by using wrapping_add or by returning an error, or we should document that these methods can overflow and make it the responsibility of the callers to check for large values.

Backtrace

 #17 0x55641b0a30ca in sspi::ber::sizeof_sequence::hed6ee39e106c7a21 /home/sduquette/git/Wayk/sspi-rs/src/ber.rs:43:4
    #18 0x55641b089a59 in sspi::credssp::ts_request::TsRequest::from_buffer::h2bf1cc361fce934b /home/sduquette/git/Wayk/sspi-rs/src/credssp/ts_request.rs:54:26
    #19 0x55641affbdc1 in rust_fuzzer_test_input /home/sduquette/git/Wayk/sspi-rs/fuzz/fuzz_targets/fuzz_ts_request.rs:8:21

@ekse I don't recall if this is still an issue

ekse commented

Yes it is still reproduceable with cargo +nightly fuzz run fuzz_ts_request.