clippy: array out-of-bound in blockmode/gcm-siv
Opened this issue · 3 comments
DuckSoft commented
error: range is out of bounds
--> src/blockmode/gcm_siv.rs:114:28
|
21 | / macro_rules! impl_block_cipher_with_gcm_siv_mode {
22 | | ($name:tt, $cipher:tt) => {
23 | |
24 | | #[derive(Clone)]
... |
114 | | ek[16..24].copy_from_slice(&tmp[0..8]);
| | ^^
... |
283 | | }
284 | | }
| |_- in this expansion of `impl_block_cipher_with_gcm_siv_mode!`
285 |
286 | impl_block_cipher_with_gcm_siv_mode!(Aes128GcmSiv, Aes128);
| ----------------------------------------------------------- in this macro invocation
|
= note: requested on the command line with `-D clippy::out-of-bounds-indexing`
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#out_of_bounds_indexing
error: range is out of bounds
--> src/blockmode/gcm_siv.rs:119:24
|
21 | / macro_rules! impl_block_cipher_with_gcm_siv_mode {
22 | | ($name:tt, $cipher:tt) => {
23 | |
24 | | #[derive(Clone)]
... |
119 | | ek[24..32].copy_from_slice(&tmp[0..8]);
| | ^^
... |
283 | | }
284 | | }
| |_- in this expansion of `impl_block_cipher_with_gcm_siv_mode!`
285 |
286 | impl_block_cipher_with_gcm_siv_mode!(Aes128GcmSiv, Aes128);
| ----------------------------------------------------------- in this macro invocation
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#out_of_bounds_indexing
error: range is out of bounds
--> src/blockmode/gcm_siv.rs:114:28
|
21 | / macro_rules! impl_block_cipher_with_gcm_siv_mode {
22 | | ($name:tt, $cipher:tt) => {
23 | |
24 | | #[derive(Clone)]
... |
114 | | ek[16..24].copy_from_slice(&tmp[0..8]);
| | ^^
... |
283 | | }
284 | | }
| |_- in this expansion of `impl_block_cipher_with_gcm_siv_mode!`
...
289 | impl_block_cipher_with_gcm_siv_mode!(Sm4GcmSiv, Sm4);
| ----------------------------------------------------- in this macro invocation
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#out_of_bounds_indexing
error: range is out of bounds
--> src/blockmode/gcm_siv.rs:119:24
|
21 | / macro_rules! impl_block_cipher_with_gcm_siv_mode {
22 | | ($name:tt, $cipher:tt) => {
23 | |
24 | | #[derive(Clone)]
... |
119 | | ek[24..32].copy_from_slice(&tmp[0..8]);
| | ^^
... |
283 | | }
284 | | }
| |_- in this expansion of `impl_block_cipher_with_gcm_siv_mode!`
...
289 | impl_block_cipher_with_gcm_siv_mode!(Sm4GcmSiv, Sm4);
| ----------------------------------------------------- in this macro invocation
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#out_of_bounds_indexing
error: range is out of bounds
--> src/blockmode/gcm_siv.rs:114:28
|
21 | / macro_rules! impl_block_cipher_with_gcm_siv_mode {
22 | | ($name:tt, $cipher:tt) => {
23 | |
24 | | #[derive(Clone)]
... |
114 | | ek[16..24].copy_from_slice(&tmp[0..8]);
| | ^^
... |
283 | | }
284 | | }
| |_- in this expansion of `impl_block_cipher_with_gcm_siv_mode!`
...
290 | impl_block_cipher_with_gcm_siv_mode!(Camellia128GcmSiv, Camellia128);
| --------------------------------------------------------------------- in this macro invocation
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#out_of_bounds_indexing
error: range is out of bounds
--> src/blockmode/gcm_siv.rs:119:24
|
21 | / macro_rules! impl_block_cipher_with_gcm_siv_mode {
22 | | ($name:tt, $cipher:tt) => {
23 | |
24 | | #[derive(Clone)]
... |
119 | | ek[24..32].copy_from_slice(&tmp[0..8]);
| | ^^
... |
283 | | }
284 | | }
| |_- in this expansion of `impl_block_cipher_with_gcm_siv_mode!`
...
290 | impl_block_cipher_with_gcm_siv_mode!(Camellia128GcmSiv, Camellia128);
| --------------------------------------------------------------------- in this macro invocation
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#out_of_bounds_indexing
error: range is out of bounds
--> src/blockmode/gcm_siv.rs:114:28
|
21 | / macro_rules! impl_block_cipher_with_gcm_siv_mode {
22 | | ($name:tt, $cipher:tt) => {
23 | |
24 | | #[derive(Clone)]
... |
114 | | ek[16..24].copy_from_slice(&tmp[0..8]);
| | ^^
... |
283 | | }
284 | | }
| |_- in this expansion of `impl_block_cipher_with_gcm_siv_mode!`
...
292 | impl_block_cipher_with_gcm_siv_mode!(Aria128GcmSiv, Aria128);
| ------------------------------------------------------------- in this macro invocation
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#out_of_bounds_indexing
error: range is out of bounds
--> src/blockmode/gcm_siv.rs:119:24
|
21 | / macro_rules! impl_block_cipher_with_gcm_siv_mode {
22 | | ($name:tt, $cipher:tt) => {
23 | |
24 | | #[derive(Clone)]
... |
119 | | ek[24..32].copy_from_slice(&tmp[0..8]);
| | ^^
... |
283 | | }
284 | | }
| |_- in this expansion of `impl_block_cipher_with_gcm_siv_mode!`
...
292 | impl_block_cipher_with_gcm_siv_mode!(Aria128GcmSiv, Aria128);
| ------------------------------------------------------------- in this macro invocation
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#out_of_bounds_indexing
error: aborting due to 8 previous errors
DuckSoft commented
cargo expand shows that the code impl_block_cipher_with_gcm_siv_mode!(Aes128GcmSiv, Aes128); generates:
#[derive(Clone)]
pub struct Aes128GcmSiv {
cipher: Aes128,
}
impl Zeroize for Aes128GcmSiv {
fn zeroize(&mut self) {
self.cipher.zeroize();
}
}
impl Drop for Aes128GcmSiv {
fn drop(&mut self) {
self.zeroize();
}
}
impl Aes128GcmSiv {
pub const KEY_LEN: usize = Aes128::KEY_LEN;
pub const BLOCK_LEN: usize = Aes128::BLOCK_LEN;
pub const NONCE_LEN: usize = 12;
pub const TAG_LEN: usize = 16;
#[cfg(target_pointer_width = "64")]
pub const A_MAX: usize = 68719476736;
#[cfg(target_pointer_width = "32")]
pub const A_MAX: usize = usize::MAX;
#[cfg(target_pointer_width = "64")]
pub const P_MAX: usize = 68719476736;
#[cfg(target_pointer_width = "32")]
pub const P_MAX: usize = usize::MAX - Self::TAG_LEN;
#[cfg(target_pointer_width = "64")]
pub const C_MAX: usize = 68719476736 + Self::TAG_LEN;
#[cfg(target_pointer_width = "32")]
pub const C_MAX: usize = usize::MAX;
pub const N_MIN: usize = Self::NONCE_LEN;
pub const N_MAX: usize = Self::NONCE_LEN;
pub fn new(key: &[u8]) -> Self {
assert!(Self::KEY_LEN == 16 || Self::KEY_LEN == 32);
assert_eq!(key.len(), Self::KEY_LEN);
assert_eq!(Self::BLOCK_LEN, GCM_SIV_BLOCK_LEN);
assert_eq!(Self::BLOCK_LEN, Polyval::BLOCK_LEN);
let cipher = Aes128::new(key);
Self { cipher }
}
#[inline]
fn derive_keys(&self, nonce: &[u8]) -> (Aes128, Polyval) {
let mut counter_block = [0u8; Self::BLOCK_LEN];
counter_block[4..16].copy_from_slice(nonce);
let mut ak = [0u8; Self::BLOCK_LEN];
let mut ek = [0u8; Self::KEY_LEN];
let mut tmp = counter_block.clone();
tmp[0] = 0;
self.cipher.encrypt(&mut tmp);
ak[0..8].copy_from_slice(&tmp[0..8]);
tmp = counter_block.clone();
tmp[0] = 1;
self.cipher.encrypt(&mut tmp);
ak[8..16].copy_from_slice(&tmp[0..8]);
tmp = counter_block.clone();
tmp[0] = 2;
self.cipher.encrypt(&mut tmp);
ek[0..8].copy_from_slice(&tmp[0..8]);
tmp = counter_block.clone();
tmp[0] = 3;
self.cipher.encrypt(&mut tmp);
ek[8..16].copy_from_slice(&tmp[0..8]);
if Self::KEY_LEN == 32 {
tmp = counter_block.clone();
tmp[0] = 4;
self.cipher.encrypt(&mut tmp);
ek[16..24].copy_from_slice(&tmp[0..8]);
tmp = counter_block.clone();
tmp[0] = 5;
self.cipher.encrypt(&mut tmp);
ek[24..32].copy_from_slice(&tmp[0..8]);
}
let cipher = Aes128::new(&ek);
let polyval = Polyval::new(&ak);
(cipher, polyval)
}
#[inline]
fn ctr32(counter_block: &mut [u8; Self::BLOCK_LEN]) {
let counter = u32::from_le_bytes([
counter_block[0], counter_block[1], counter_block[2], counter_block[3]
]);
counter_block[0..4].copy_from_slice(&counter.wrapping_add(1).to_le_bytes());
}
pub fn encrypt_slice(&self, nonce: &[u8], aad: &[u8], aead_pkt: &mut [u8]) {
debug_assert!(aead_pkt.len() >= Self::TAG_LEN);
let plen = aead_pkt.len() - Self::TAG_LEN;
let (plaintext_in_ciphertext_out, tag_out) = aead_pkt.split_at_mut(plen);
self.encrypt_slice_detached(nonce, aad, plaintext_in_ciphertext_out, tag_out)
}
pub fn decrypt_slice(&self, nonce: &[u8], aad: &[u8], aead_pkt: &mut [u8]) -> bool {
debug_assert!(aead_pkt.len() >= Self::TAG_LEN);
let clen = aead_pkt.len() - Self::TAG_LEN;
let (ciphertext_in_plaintext_out, tag_in) = aead_pkt.split_at_mut(clen);
self.decrypt_slice_detached(nonce, aad, ciphertext_in_plaintext_out, &tag_in)
}
pub fn encrypt_slice_detached(&self, nonce: &[u8], aad: &[u8], plaintext_in_ciphertext_out: &mut [u8], tag_out: &mut [u8]) {
assert_eq!(nonce.len(), Self::NONCE_LEN);
let alen = aad.len();
let plen = plaintext_in_ciphertext_out.len();
let tlen = tag_out.len();
debug_assert!(alen <= Self::A_MAX);
debug_assert!(plen <= Self::P_MAX);
debug_assert!(tlen == Self::TAG_LEN);
let (cipher, mut polyval) = self.derive_keys(nonce);
let mut bit_len_block = [0u8; Self::BLOCK_LEN];
bit_len_block[0..8].copy_from_slice(&(alen as u64 * 8).to_le_bytes());
bit_len_block[8..16].copy_from_slice(&(plen as u64 * 8).to_le_bytes());
polyval.update(aad);
polyval.update(&plaintext_in_ciphertext_out);
polyval.update(&bit_len_block);
let mut tag = polyval.finalize();
for i in 0..Self::NONCE_LEN {
tag[i] ^= nonce[i];
}
tag[15] &= 0x7f;
cipher.encrypt(&mut tag);
let mut counter_block = tag.clone();
counter_block[15] |= 0x80;
let n = plen / Self::BLOCK_LEN;
for i in 0..n {
let mut keystream_block = counter_block.clone();
cipher.encrypt(&mut keystream_block);
Self::ctr32(&mut counter_block);
let block = &mut plaintext_in_ciphertext_out[i * Self::BLOCK_LEN..i * Self::BLOCK_LEN + Self::BLOCK_LEN];
xor_si128_inplace(block, &keystream_block);
}
if plen % Self::BLOCK_LEN != 0 {
let mut keystream_block = counter_block.clone();
cipher.encrypt(&mut keystream_block);
Self::ctr32(&mut counter_block);
let rem = &mut plaintext_in_ciphertext_out[n * Self::BLOCK_LEN..];
for i in 0..rem.len() {
rem[i] ^= keystream_block[i];
}
}
tag_out.copy_from_slice(&tag[..Self::TAG_LEN]);
}
pub fn decrypt_slice_detached(&self, nonce: &[u8], aad: &[u8], ciphertext_in_plaintext_out: &mut [u8], tag_in: &[u8]) -> bool {
assert_eq!(nonce.len(), Self::NONCE_LEN);
let alen = aad.len();
let clen = ciphertext_in_plaintext_out.len();
let tlen = tag_in.len();
debug_assert!(alen <= Self::A_MAX);
debug_assert!(clen <= Self::P_MAX);
debug_assert!(tlen == Self::TAG_LEN);
let (cipher, mut polyval) = self.derive_keys(nonce);
let mut counter_block = [0u8; Self::BLOCK_LEN];
counter_block.copy_from_slice(&tag_in);
counter_block[15] |= 0x80;
let n = clen / Self::BLOCK_LEN;
for i in 0..n {
let mut keystream_block = counter_block.clone();
cipher.encrypt(&mut keystream_block);
Self::ctr32(&mut counter_block);
let block = &mut ciphertext_in_plaintext_out[i * Self::BLOCK_LEN..i * Self::BLOCK_LEN + Self::BLOCK_LEN];
xor_si128_inplace(block, &keystream_block);
}
if clen % Self::BLOCK_LEN != 0 {
let mut keystream_block = counter_block.clone();
cipher.encrypt(&mut keystream_block);
Self::ctr32(&mut counter_block);
let rem = &mut ciphertext_in_plaintext_out[n * Self::BLOCK_LEN..];
for i in 0..rem.len() {
rem[i] ^= keystream_block[i];
}
}
let mut bit_len_block = [0u8; Self::BLOCK_LEN];
bit_len_block[0..8].copy_from_slice(&(alen as u64 * 8).to_le_bytes());
bit_len_block[8..16].copy_from_slice(&(clen as u64 * 8).to_le_bytes());
polyval.update(aad);
polyval.update(&ciphertext_in_plaintext_out);
polyval.update(&bit_len_block);
let mut tag = polyval.finalize();
for i in 0..Self::NONCE_LEN {
tag[i] ^= nonce[i];
}
tag[15] &= 0x7f;
cipher.encrypt(&mut tag);
constant_time_eq(tag_in, &tag[..Self::TAG_LEN])
}
}where there is:
if Self::KEY_LEN == 32 {
tmp = counter_block.clone();
tmp[0] = 4;
self.cipher.encrypt(&mut tmp);
ek[16..24].copy_from_slice(&tmp[0..8]);
tmp = counter_block.clone();
tmp[0] = 5;
self.cipher.encrypt(&mut tmp);
ek[24..32].copy_from_slice(&tmp[0..8]);
}since Self::KEY_LEN == 16, that if block shall not exist (at all) in the generated code.
I suggest modify the macro_rules to fix the issue.
DuckSoft commented
Update: the warning is temporarily suppressed by 993f47a#diff-f7b53ee42d9d571f6f3a8ed268b8bcc71ba0306507c88194c183695721cfd829R80.
Don't forget to fix it.
zonyitoo commented
It doesn't affect correctness. But we should find a way to make clippy happy.