Nitrokey / nitrokey-3-firmware

Nitrokey 3 firmware
Apache License 2.0
248 stars 25 forks source link

usbd-ctaphid crashes while handling big message #159

Closed szszszsz closed 1 year ago

szszszsz commented 1 year ago

usbd-ctaphid crate crashes while handling big message.

Found while executing load tests for oath-app over usb-ip simulation:

Note: check first, if the affected dependency is not outdated.

log ```rust thread 'main' panicked at 'range end index 7542 out of range for slice of length 3072', /home/sz/work/webcrypt/webcrypt -usbip/components/usbd-ctaphid/src/pipe.rs:570:25 stack backtrace: 0: rust_begin_unwind at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/std/src/panicking.rs:584:5 1: core::panicking::panic_fmt at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/core/src/panicking.rs:142:14 2: core::slice::index::slice_end_index_len_fail_rt at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/core/src/slice/index.rs:76:5 3: core::slice::index::slice_end_index_len_fail at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/core/src/slice/index.rs:69:9 4: as core::slice::index::SliceIndex<[T]>>::index_mut at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/core/src/slice/index.rs:327:13 5: as core::slice::index::SliceIndex<[T]>>::index_mut at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/core/src/slice/index.rs:368:9 6: core::slice::index:: for [T]>::index_mut at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/core/src/slice/index.rs:30:9 7: core::array:: for [T; N]>::index_mut at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/core/src/array/mod.rs:309:9 8: usbd_ctaphid::pipe::Pipe::handle_response at ./components/usbd-ctaphid/src/pipe.rs:570:25 9: as usb_device::class::UsbClass>::poll at ./components/usbd-ctaphid/src/class.rs:199:9 10: usb_device::device::UsbDevice::poll at /home/sz/.cargo/registry/src/github.com-1ecc6299db9ec823/usb-device-0.2.9/src/device.rs:273:21 11: usbip_simulation::main at ./src/bin/main.rs:186:9 12: core::ops::function::FnOnce::call_once at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/core/src/ops/function.rs:248:5 note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace. ```

Edit: note: limiting reply's length avoids sim crash

robin-nitrokey commented 1 year ago

The maximum length for messages handled by usbd-ctaphid is MESSAGE_SIZE = 3072 while ctaphid-dispatch allows up to 7609 bytes. Pipe::handle_response fails to ensure that the response returned by the application respects this limit. It should return ERR_OTHER instead.

robin-nitrokey commented 1 year ago

This PR avoids the panic and returns an error instead: https://github.com/trussed-dev/usbd-ctaphid/pull/2