From 6ffb74d49194894f70410875e9971e65a8bb0220 Mon Sep 17 00:00:00 2001
From: Max Dymond <max.dymond@microsoft.com>
Date: Tue, 1 Mar 2022 16:15:40 +0000
Subject: [PATCH] `rust-crypto`: miscomputation when performing AES encryption
 (#1201)

---
 crates/rust-crypto/RUSTSEC-0000-0000.md | 132 ++++++++++++++++++++++++
 1 file changed, 132 insertions(+)
 create mode 100644 crates/rust-crypto/RUSTSEC-0000-0000.md

diff --git a/crates/rust-crypto/RUSTSEC-0000-0000.md b/crates/rust-crypto/RUSTSEC-0000-0000.md
new file mode 100644
index 0000000..f3de024
--- /dev/null
+++ b/crates/rust-crypto/RUSTSEC-0000-0000.md
@@ -0,0 +1,132 @@
+```toml
+[advisory]
+id = "RUSTSEC-0000-0000"
+
+package = "rust-crypto"
+
+date = "2022-02-28"
+
+categories = ["crypto-failure"]
+
+keywords = ["aesni"]
+
+[versions]
+patched = []
+```
+
+# Miscomputation when performing AES encryption in rust-crypto
+
+The following Rust program demonstrates some strangeness in AES encryption - if you have an immutable key slice and then operate on that slice, you get different encryption output than if you operate on a copy of that key.
+
+For these functions, we expect that extending a 16 byte key to a 32 byte key by repeating it gives the same encrypted data, because the underlying rust-crypto functions repeat key data up to the necessary key size for the cipher.
+
+```rust
+use crypto::{
+    aes, blockmodes, buffer,
+    buffer::{BufferResult, ReadBuffer, WriteBuffer},
+    symmetriccipher,
+};
+
+fn encrypt(
+    key: &[u8],
+    iv: &[u8],
+    data: &str,
+) -> Result<String, symmetriccipher::SymmetricCipherError> {
+    let mut encryptor =
+        aes::cbc_encryptor(aes::KeySize::KeySize256, key, iv, blockmodes::PkcsPadding);
+
+    let mut encrypted_data = Vec::<u8>::new();
+    let mut read_buffer = buffer::RefReadBuffer::new(data.as_bytes());
+    let mut buffer = [0; 4096];
+    let mut write_buffer = buffer::RefWriteBuffer::new(&mut buffer);
+
+    loop {
+        let result = encryptor.encrypt(&mut read_buffer, &mut write_buffer, true)?;
+
+        encrypted_data.extend(
+            write_buffer
+                .take_read_buffer()
+                .take_remaining()
+                .iter()
+                .copied(),
+        );
+
+        match result {
+            BufferResult::BufferUnderflow => break,
+            BufferResult::BufferOverflow => {}
+        }
+    }
+
+    Ok(hex::encode(encrypted_data))
+}
+
+fn working() {
+    let data = "data";
+    let iv = [
+        0xF0, 0xF1, 0xF2, 0xF3, 0xF4, 0xF5, 0xF6, 0xF7, 0xF8, 0xF9, 0xFA, 0xFB, 0xFC, 0xFD, 0xFE,
+        0xFF,
+    ];
+    let key = [
+        0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E,
+        0x0F,
+    ];
+    // The copy here makes the code work.
+    let key_copy = key;
+    let key2: Vec<u8> = key_copy.iter().cycle().take(32).copied().collect();
+    println!("key1:{} key2: {}", hex::encode(&key), hex::encode(&key2));
+
+    let x1 = encrypt(&key, &iv, data).unwrap();
+    println!("X1: {}", x1);
+
+    let x2 = encrypt(&key2, &iv, data).unwrap();
+    println!("X2: {}", x2);
+
+    assert_eq!(x1, x2);
+}
+
+fn broken() {
+    let data = "data";
+    let iv = [
+        0xF0, 0xF1, 0xF2, 0xF3, 0xF4, 0xF5, 0xF6, 0xF7, 0xF8, 0xF9, 0xFA, 0xFB, 0xFC, 0xFD, 0xFE,
+        0xFF,
+    ];
+    let key = [
+        0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E,
+        0x0F,
+    ];
+    // This operation shouldn't affect the contents of key at all.
+    let key2: Vec<u8> = key.iter().cycle().take(32).copied().collect();
+    println!("key1:{} key2: {}", hex::encode(&key), hex::encode(&key2));
+
+    let x1 = encrypt(&key, &iv, data).unwrap();
+    println!("X1: {}", x1);
+
+    let x2 = encrypt(&key2, &iv, data).unwrap();
+    println!("X2: {}", x2);
+
+    assert_eq!(x1, x2);
+}
+
+fn main() {
+    working();
+    broken();
+}
+```
+
+The output from this program:
+
+```shell
+     Running `target/host/debug/rust-crypto-test`
+key1:000102030405060708090a0b0c0d0e0f key2: 000102030405060708090a0b0c0d0e0f000102030405060708090a0b0c0d0e0f
+X1: 90462bbe32965c8e7ea0addbbed4cddb
+X2: 90462bbe32965c8e7ea0addbbed4cddb
+key1:000102030405060708090a0b0c0d0e0f key2: 000102030405060708090a0b0c0d0e0f000102030405060708090a0b0c0d0e0f
+X1: 26e847e5e7df1947bf82a650548a7d5b
+X2: 90462bbe32965c8e7ea0addbbed4cddb
+thread 'main' panicked at 'assertion failed: `(left == right)`
+  left: `"26e847e5e7df1947bf82a650548a7d5b"`,
+ right: `"90462bbe32965c8e7ea0addbbed4cddb"`', src/main.rs:83:5
+```
+
+Notably, the X1 key in the `broken()` test changes every time after rerunning the program.
+