From 607e069b33bfdbcb9f27ff670c7a29e889b66499 Mon Sep 17 00:00:00 2001
From: Thomas Fussell <thomas.fussell@gmail.com>
Date: Wed, 19 Apr 2017 16:00:33 -0400
Subject: [PATCH] fix two crypto related todos

---
 source/detail/crypto/aes.cpp         | 46 ++++++++++++++++------------
 source/detail/crypto/aes.hpp         | 12 +++++---
 source/detail/crypto/xlsx_crypto.cpp | 33 ++++++++------------
 3 files changed, 48 insertions(+), 43 deletions(-)

diff --git a/source/detail/crypto/aes.cpp b/source/detail/crypto/aes.cpp
index 94806f87..d37461ae 100755
--- a/source/detail/crypto/aes.cpp
+++ b/source/detail/crypto/aes.cpp
@@ -1334,19 +1334,21 @@ namespace detail {
 
 std::vector<std::uint8_t> aes_ecb_encrypt(
     const std::vector<std::uint8_t> &plaintext,
-    const std::vector<std::uint8_t> &key)
+    const std::vector<std::uint8_t> &key,
+    const std::size_t offset)
 {
     if (plaintext.empty()) return {};
 
-    if (plaintext.size() % 16 != 0)
+    auto len = plaintext.size() - offset;
+
+    if (len % 16 != 0)
     {
         throw std::runtime_error("");
     }
 
-    auto ciphertext = std::vector<std::uint8_t>(plaintext.size());
+    auto ciphertext = std::vector<std::uint8_t>(len);
     auto expanded_key = rijndael_setup(key);
-    auto len = plaintext.size();
-    auto pt = plaintext.data();
+    auto pt = plaintext.data() + offset;
     auto ct = ciphertext.data();
 
     while (len)
@@ -1363,19 +1365,21 @@ std::vector<std::uint8_t> aes_ecb_encrypt(
 
 std::vector<std::uint8_t> aes_ecb_decrypt(
     const std::vector<std::uint8_t> &ciphertext,
-    const std::vector<std::uint8_t> &key)
+    const std::vector<std::uint8_t> &key,
+    const std::size_t offset)
 {
     if (ciphertext.empty()) return {};
 
-    if (ciphertext.size() % 16 != 0)
+    auto len = ciphertext.size() - offset;
+
+    if (len % 16 != 0)
     {
         throw std::runtime_error("");
     }
 
-    auto plaintext = std::vector<std::uint8_t>(ciphertext.size());
+    auto plaintext = std::vector<std::uint8_t>(len);
     auto expanded_key = rijndael_setup(key);
-    auto len = ciphertext.size();
-    auto ct = ciphertext.data();
+    auto ct = ciphertext.data() + offset;
     auto pt = plaintext.data();
 
     while (len)
@@ -1393,20 +1397,22 @@ std::vector<std::uint8_t> aes_ecb_decrypt(
 std::vector<std::uint8_t> aes_cbc_encrypt(
     const std::vector<std::uint8_t> &plaintext,
     const std::vector<std::uint8_t> &key,
-    const std::vector<std::uint8_t> &original_iv)
+    const std::vector<std::uint8_t> &original_iv,
+    const std::size_t offset)
 {
     if (plaintext.empty()) return {};
 
-    if (plaintext.size() % 16 != 0)
+    auto len = plaintext.size() - offset;
+
+    if (len % 16 != 0)
     {
         throw std::runtime_error("");
     }
 
-    auto ciphertext = std::vector<std::uint8_t>(plaintext.size());
+    auto ciphertext = std::vector<std::uint8_t>(len);
     auto expanded_key = rijndael_setup(key);
-    auto len = plaintext.size();
     auto ct = ciphertext.data();
-    auto pt = plaintext.data();
+    auto pt = plaintext.data() + offset;
     auto iv_vec = original_iv;
     auto iv = iv_vec.data();
 
@@ -1435,10 +1441,13 @@ std::vector<std::uint8_t> aes_cbc_encrypt(
 std::vector<std::uint8_t> aes_cbc_decrypt(
     const std::vector<std::uint8_t> &ciphertext,
     const std::vector<std::uint8_t> &key,
-    const std::vector<std::uint8_t> &original_iv)
+    const std::vector<std::uint8_t> &original_iv,
+    const std::size_t offset)
 {
     if (ciphertext.empty()) return {};
 
+    auto len = ciphertext.size() - offset;
+
     if (ciphertext.size() % 16 != 0)
     {
         throw std::runtime_error("");
@@ -1446,10 +1455,9 @@ std::vector<std::uint8_t> aes_cbc_decrypt(
 
     std::array<std::uint8_t, 16> temporary{{0}};
 
-    auto plaintext = std::vector<std::uint8_t>(ciphertext.size());
+    auto plaintext = std::vector<std::uint8_t>(len);
     auto expanded_key = rijndael_setup(key);
-    auto len = ciphertext.size();
-    auto ct = ciphertext.data();
+    auto ct = ciphertext.data() + offset;
     auto pt = plaintext.data();
     auto iv_vec = original_iv;
     auto iv = iv_vec.data();
diff --git a/source/detail/crypto/aes.hpp b/source/detail/crypto/aes.hpp
index c192e559..e1657bdb 100755
--- a/source/detail/crypto/aes.hpp
+++ b/source/detail/crypto/aes.hpp
@@ -31,21 +31,25 @@ namespace detail {
 
 std::vector<std::uint8_t> aes_ecb_encrypt(
     const std::vector<std::uint8_t> &input,
-    const std::vector<std::uint8_t> &key);
+    const std::vector<std::uint8_t> &key,
+    const std::size_t offset = 0);
 
 std::vector<std::uint8_t> aes_ecb_decrypt(
     const std::vector<std::uint8_t> &input,
-    const std::vector<std::uint8_t> &key);
+    const std::vector<std::uint8_t> &key,
+    const std::size_t offset = 0);
 
 std::vector<std::uint8_t> aes_cbc_encrypt(
     const std::vector<std::uint8_t> &input,
     const std::vector<std::uint8_t> &key,
-    const std::vector<std::uint8_t> &iv);
+    const std::vector<std::uint8_t> &iv,
+    const std::size_t offset = 0);
 
 std::vector<std::uint8_t> aes_cbc_decrypt(
     const std::vector<std::uint8_t> &input,
     const std::vector<std::uint8_t> &key,
-    const std::vector<std::uint8_t> &iv);
+    const std::vector<std::uint8_t> &iv,
+    const std::size_t offset = 0);
 
 } // namespace detail
 } // namespace xlnt
diff --git a/source/detail/crypto/xlsx_crypto.cpp b/source/detail/crypto/xlsx_crypto.cpp
index 87a95378..ca43fdff 100644
--- a/source/detail/crypto/xlsx_crypto.cpp
+++ b/source/detail/crypto/xlsx_crypto.cpp
@@ -240,9 +240,15 @@ std::vector<std::uint8_t> decrypt_xlsx_standard(
     offset += verifier_size;
 
     const auto verifier_hash_size = read_int<std::uint32_t>(offset, encryption_info);
+    const auto encrypted_verifier_hash_size = std::size_t(32);
     std::vector<std::uint8_t> encrypted_verifier_hash(encryption_info.begin() + static_cast<std::ptrdiff_t>(offset),
-        encryption_info.begin() + static_cast<std::ptrdiff_t>(offset + verifier_hash_size));
-    offset += verifier_hash_size;
+        encryption_info.begin() + static_cast<std::ptrdiff_t>(offset + encrypted_verifier_hash_size));
+    offset += encrypted_verifier_hash_size;
+
+    if (offset != encryption_info.size())
+    {
+        throw xlnt::exception("extra data after encryption info");
+    }
 
     // begin key generation algorithm
 
@@ -301,32 +307,19 @@ std::vector<std::uint8_t> decrypt_xlsx_standard(
 
     auto calculated_verifier_hash = hash(info.hash,
         aes_ecb_decrypt(encrypted_verifier, key));
-
-    // TODO: I can't figure out how to pad encrypted_verifier_hash to 32 bytes
-    // so that it matches calculated_verifier_hash after decryption so we'll just 
-    // compare the first block (16 bytes) for now.
-    calculated_verifier_hash.resize(16);
-    encrypted_verifier_hash.resize(16);
-
     auto decrypted_verifier_hash = aes_ecb_decrypt(
         encrypted_verifier_hash, key);
+    decrypted_verifier_hash.resize(verifier_hash_size);
 
     if (calculated_verifier_hash != decrypted_verifier_hash)
     {
         throw xlnt::exception("bad password");
     }
 
-    std::size_t ignore = 0;
-    auto decrypted_size = static_cast<std::size_t>(
-        read_int<std::uint64_t>(ignore, encrypted_package));
-
-    // TODO: don't copy encrypted package just to cut off the first 8 bytes
-    auto encrypted_data = std::vector<std::uint8_t>(
-        encrypted_package.begin() + 8, 
-        encrypted_package.end());
-    auto decrypted = aes_ecb_decrypt(encrypted_data, key);
-
-    decrypted.resize(decrypted_size);
+    offset = 0;
+    auto decrypted_size = read_int<std::uint64_t>(offset, encrypted_package);
+    auto decrypted = aes_ecb_decrypt(encrypted_package, key, offset);
+    decrypted.resize(static_cast<std::size_t>(decrypted_size));
 
     return decrypted;
 }