Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cipher: StreamCipherCoreWrapper::get_pos() with a block size of 0 can reach unreachable_unchecked() #1275

Closed
LegionMammal978 opened this issue Mar 8, 2023 · 1 comment · Fixed by #1277

Comments

@LegionMammal978
Copy link

The safety comment in the function says "pos is set only to values smaller than block size", but if the block size of the StreamCipherCore is equal to 0 (in a contrived scenario), then initializing pos to 0 breaks this invariant. When debug assertions are disabled, this can cause get_pos() to reach the unreachable_unchecked(). This can be seen with Miri:

// RUSTFLAGS=-Cdebug-assertions=off cargo +nightly miri run
// cipher = "=0.4.3"
use cipher::{
    consts::U0, BlockSizeUser, StreamCipher, StreamCipherCore, StreamCipherCoreWrapper,
    StreamClosure,
};

struct Dummy;

impl BlockSizeUser for Dummy {
    type BlockSize = U0;
}

impl StreamCipherCore for Dummy {
    fn remaining_blocks(&self) -> Option<usize> {
        None
    }

    fn process_with_backend(&mut self, _: impl StreamClosure<BlockSize = Self::BlockSize>) {
        unimplemented!()
    }
}

fn main() {
    let mut wrapper = StreamCipherCoreWrapper::from_core(Dummy);
    wrapper.apply_keystream(&mut []);
}
error: Undefined Behavior: entering unreachable code
  --> /home/lm978/.cargo/registry/src/github.com-1ecc6299db9ec823/cipher-0.4.3/src/stream_wrapper.rs:53:22
   |
53 |             unsafe { core::hint::unreachable_unchecked() }
   |                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ entering unreachable code
   |
   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
   = note: BACKTRACE:
   = note: inside `cipher::StreamCipherCoreWrapper::<Dummy>::get_pos` at /home/lm978/.cargo/registry/src/github.com-1ecc6299db9ec823/cipher-0.4.3/src/stream_wrapper.rs:53:22: 53:57
   = note: inside `<cipher::StreamCipherCoreWrapper<Dummy> as cipher::StreamCipher>::try_apply_keystream_inout` at /home/lm978/.cargo/registry/src/github.com-1ecc6299db9ec823/cipher-0.4.3/src/stream_wrapper.rs:118:19: 118:33
   = note: inside `<cipher::StreamCipherCoreWrapper<Dummy> as cipher::StreamCipher>::try_apply_keystream` at /home/lm978/.cargo/registry/src/github.com-1ecc6299db9ec823/cipher-0.4.3/src/stream.rs:94:9: 94:51
   = note: inside `<cipher::StreamCipherCoreWrapper<Dummy> as cipher::StreamCipher>::apply_keystream` at /home/lm978/.cargo/registry/src/github.com-1ecc6299db9ec823/cipher-0.4.3/src/stream.rs:120:9: 120:38
note: inside `main`
  --> src/main.rs:26:5
   |
26 |     wrapper.apply_keystream(&mut []);
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

error: aborting due to previous error

Perhaps a defensive guard against the BlockSize = U0 case could be added somewhere.

@LegionMammal978 LegionMammal978 changed the title StreamCipherCoreWrapper::get_pos() with a block size of 0 can reach unreachable_unchecked() cipher: StreamCipherCoreWrapper::get_pos() with a block size of 0 can reach unreachable_unchecked() Mar 8, 2023
@newpavlov
Copy link
Member

Thank you for reporting this! In cipher v0.5 it will be fixed by the BlockSizes trait, but for now I will add a panic for zero block sizes before the branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants