Merge branch 'explicit-bounds'

Harden against out-of-bounds writes.

Chaos/Cipher/Block/Decryptor.hpp

@@ -9,9 +9,10 @@ class Decryptor
 {
 public:
     template<typename OutputIt, typename InputIt>
-    void DecryptBlock(OutputIt out, InputIt inBegin, InputIt inEnd) const
+    void DecryptBlock(OutputIt outBegin, OutputIt outEnd,
+                      InputIt inBegin, InputIt inEnd) const
     {
-        Impl().DecryptBlock(out, inBegin, inEnd);
+        Impl().DecryptBlock(outBegin, outEnd, inBegin, inEnd);
     }
 
     template<typename Block>

Chaos/Cipher/Block/Des/DesCrypt.hpp

@@ -83,11 +83,12 @@ struct Bitwise
     }
 
     template<typename OutputIt>
-    static void CrunchUInt64(OutputIt out, uint64_t value)
+    static void CrunchUInt64(OutputIt outBegin, OutputIt outEnd, uint64_t value)
     {
-        for (int_fast8_t i = 0; i < 8; ++i)
+        int_fast8_t i = 0;
+        for (OutputIt out = outBegin; i < 8 && out != outEnd; ++i, ++out)
         {
-            *out++ = (value >> (56 - (i * 8))) & Mask<8>();
+            *out = (value >> (56 - (i * 8))) & Mask<8>();
         }
     }
 };
@@ -237,7 +238,8 @@ public:
         { }
 
         template<typename OutputIt, typename InputIt>
-        void EncryptBlock(OutputIt out, InputIt inBegin, InputIt inEnd) const
+        void EncryptBlock(OutputIt outBegin, OutputIt outEnd,
+                          InputIt inBegin, InputIt inEnd) const
         {
             RawBlockArray block;
 
@@ -252,7 +254,7 @@ public:
                                                                      block.End()),
                                          Schedule_);
 
-            Inner_::Bitwise::CrunchUInt64(out, encrypted);
+            Inner_::Bitwise::CrunchUInt64(outBegin, outEnd, encrypted);
         }
 
         Block EncryptBlock(Block block) const
@@ -279,7 +281,8 @@ public:
         { }
 
         template<typename OutputIt, typename InputIt>
-        void DecryptBlock(OutputIt out, InputIt inBegin, InputIt inEnd) const
+        void DecryptBlock(OutputIt outBegin, OutputIt outEnd,
+                          InputIt inBegin, InputIt inEnd) const
         {
             RawBlockArray block;
 
@@ -294,7 +297,7 @@ public:
                                                                      block.End()),
                                          Schedule_);
 
-            Inner_::Bitwise::CrunchUInt64(out, decrypted);
+            Inner_::Bitwise::CrunchUInt64(outBegin, outEnd, decrypted);
         }
 
         Block DecryptBlock(Block block) const

Chaos/Cipher/Block/Encryptor.hpp

@@ -9,9 +9,10 @@ class Encryptor
 {
 public:
     template<typename OutputIt, typename InputIt>
-    void EncryptBlock(OutputIt out, InputIt inBegin, InputIt inEnd) const
+    void EncryptBlock(OutputIt outBegin, OutputIt outEnd,
+                      InputIt inBegin, InputIt inEnd) const
     {
-        Impl().EncryptBlock(out, inBegin, inEnd);
+        Impl().EncryptBlock(outBegin, outEnd, inBegin, inEnd);
     }
 
     template<typename Block>

ChaosTests/Cipher/Arc4CryptTests.cpp

@@ -107,3 +107,125 @@ TEST(Arc4CryptTests, RekeyTest)
                   ciphertext);
     }
 }
+
+TEST(Arc4CryptTests, EncryptOutIteratorUsageTest)
+{
+    const std::vector<uint8_t> data = StrToU8Vec("The quick brown fox jumps over the lazy dog.");
+
+    {
+        std::array<uint8_t, 5> key = { 0x01, 0x02, 0x03, 0x04, 0x05 };
+
+        Arc4Crypt crypt(key.begin(), key.end());
+
+        std::array<uint8_t, 47> out;
+        out.fill(0);
+
+        std::array<uint8_t, 47> expected =
+        {
+            0x00, 0x00, 0x00,
+            0xe6, 0x51, 0x06, 0x25, 0x81, 0x48, 0xa9, 0x44, 0xa7, 0xe3, 0x30,
+            0x38, 0x65, 0x66, 0x76, 0x88, 0x0f, 0xed, 0xec, 0x6f, 0x72, 0x89,
+            0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+            0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+        };
+
+        crypt.Encrypt(out.begin() + 3, data.begin(), 22);
+
+        ASSERT_EQ(expected, out);
+    }
+
+    {
+        std::array<uint8_t, 5> key = { 0x01, 0x02, 0x03, 0x04, 0x05 };
+
+        Arc4Crypt crypt(key.begin(), key.end());
+
+        std::array<uint8_t, 47> out;
+        out.fill(0);
+
+        std::array<uint8_t, 47> expected =
+        {
+            0x00, 0x00, 0x00,
+            0xe6, 0x51, 0x06, 0x25, 0x81, 0x48, 0xa9, 0x44, 0xa7, 0xe3, 0x30,
+            0x38, 0x65, 0x66, 0x76, 0x88, 0x0f, 0xed, 0xec, 0x6f, 0x72, 0x89,
+            0xef, 0xa5, 0xfa, 0xe4, 0x6c, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+            0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+        };
+
+        crypt.Encrypt(out.begin() + 3, data.begin(), 27);
+
+        ASSERT_EQ(expected, out);
+    }
+
+    {
+        std::array<uint8_t, 5> key = { 0x01, 0x02, 0x03, 0x04, 0x05 };
+
+        Arc4Crypt crypt(key.begin(), key.end());
+
+        std::array<uint8_t, 44> out;
+        out.fill(0);
+
+        std::array<uint8_t, 44> expected;
+        expected.fill(0);
+
+        crypt.Encrypt(out.begin() + 3, data.begin(), 0);
+
+        ASSERT_EQ(expected, out);
+    }
+}
+
+TEST(Arc4CryptTests, DecryptOutIteratorUsageTest)
+{
+    const std::array<uint8_t, 14> data = { 0x45, 0xA0, 0x1F, 0x64, 0x5F, 0xC3, 0x5B,
+                                           0x38, 0x35, 0x52, 0x54, 0x4B, 0x9B, 0xF5 };
+    const std::vector<uint8_t> key = StrToU8Vec("Secret");
+
+    {
+        Arc4Crypt crypt(key.begin(), key.end());
+
+        std::array<uint8_t, 17> out;
+        out.fill(0);
+
+        std::array<uint8_t, 17> expected =
+        {
+            0x00, 0x00, 0x00,
+            'A', 't', 't', 'a', 'c', 'k', ' ', 'a', 't', ' ', 'd', 'a',
+            0x00, 0x00
+        };
+
+        crypt.Decrypt(out.begin() + 3, data.begin(), 12);
+
+        ASSERT_EQ(expected, out);
+    }
+
+    {
+        Arc4Crypt crypt(key.begin(), key.end());
+
+        std::array<uint8_t, 17> out;
+        out.fill(0);
+
+        std::array<uint8_t, 17> expected =
+        {
+            0x00, 0x00, 0x00,
+            'A', 't', 't', 'a', 'c', 'k', ' ',
+            0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+        };
+
+        crypt.Decrypt(out.begin() + 3, data.begin(), 7);
+
+        ASSERT_EQ(expected, out);
+    }
+
+    {
+        Arc4Crypt crypt(key.begin(), key.end());
+
+        std::array<uint8_t, 14> out;
+        out.fill(0);
+
+        std::array<uint8_t, 14> expected;
+        expected.fill(0);
+
+        crypt.Decrypt(out.begin() + 3, data.begin(), 0);
+
+        ASSERT_EQ(expected, out);
+    }
+}

ChaosTests/Cipher/Arc4GenTests.cpp

@@ -353,3 +353,61 @@ TEST(Arc4GenTests, UninitializedGenTest)
         ASSERT_THROW(gen.Drop(256), Chaos::Service::ChaosException);
     }
 }
+
+TEST(Arc4GenTests, GenerateOutIteratorUsageTest)
+{
+    {
+        uint8_t key[] = { 0x01, 0x02, 0x03, 0x04, 0x05 };
+        Arc4Gen gen(key, key + std::size(key));
+
+        std::array<uint8_t, 23> out;
+        out.fill(0);
+
+        std::array<uint8_t, 23> expected =
+        {
+            0x00, 0x00, 0x00,
+            0xb2, 0x39, 0x63, 0x05, 0xf0, 0x3d, 0xc0, 0x27,
+            0xcc, 0xc3, 0x52, 0x00, 0x00, 0x00, 0x00, 0x00,
+            0x00, 0x00, 0x00, 0x00
+        };
+
+        gen.Generate(out.begin() + 3, 11);
+
+        ASSERT_EQ(expected, out);
+    }
+
+    {
+        uint8_t key[] = { 0x01, 0x02, 0x03, 0x04, 0x05 };
+        Arc4Gen gen(key, key + std::size(key));
+
+        std::array<uint8_t, 23> out;
+        out.fill(0);
+
+        std::array<uint8_t, 23> expected =
+        {
+            0x00, 0x00, 0x00,
+            0xb2, 0x39, 0x63, 0x05, 0xf0, 0x3d, 0xc0, 0x27,
+            0xcc, 0xc3, 0x52, 0x4a, 0x0a, 0x11, 0x18, 0xa8,
+            0x69, 0x82, 0x00, 0x00
+        };
+
+        gen.Generate(out.begin() + 3, 18);
+
+        ASSERT_EQ(expected, out);
+    }
+
+    {
+        uint8_t key[] = { 0x01, 0x02, 0x03, 0x04, 0x05 };
+        Arc4Gen gen(key, key + std::size(key));
+
+        std::array<uint8_t, 20> out;
+        out.fill(0);
+
+        std::array<uint8_t, 20> expected;
+        expected.fill(0);
+
+        gen.Generate(out.begin() + 3, 0);
+
+        ASSERT_EQ(expected, out);
+    }
+}

ChaosTests/Cipher/DesCryptTests.cpp

@@ -51,7 +51,7 @@ TEST(DesCryptTests, EncryptTest)
 
             DesCrypt::Key desKey(key.begin(), key.end());
             DesCrypt::DesEncryptor enc(desKey);
-            enc.EncryptBlock(result.begin(), data.begin(), data.end());
+            enc.EncryptBlock(result.begin(), result.end(), data.begin(), data.end());
 
             return result;
         }
@@ -143,7 +143,7 @@ TEST(DesCryptTests, EncryptShortDataTest)
 
             DesCrypt::Key desKey(key.begin(), key.end());
             DesCrypt::DesEncryptor enc(desKey);
-            enc.EncryptBlock(result.begin(), data.begin(), data.end());
+            enc.EncryptBlock(result.begin(), result.end(), data.begin(), data.end());
 
             return result;
         }
@@ -177,7 +177,7 @@ TEST(DesCryptTests, EncryptLongDataTest)
 
             DesCrypt::Key desKey(key.begin(), key.end());
             DesCrypt::DesEncryptor enc(desKey);
-            enc.EncryptBlock(result.begin(), data.begin(), data.end());
+            enc.EncryptBlock(result.begin(), result.end(), data.begin(), data.end());
 
             return result;
         }
@@ -211,7 +211,7 @@ TEST(DesCryptTests, DecryptTest)
 
             DesCrypt::Key desKey(key.begin(), key.end());
             DesCrypt::DesDecryptor dec(desKey);
-            dec.DecryptBlock(result.begin(), data.begin(), data.end());
+            dec.DecryptBlock(result.begin(), result.end(), data.begin(), data.end());
 
             return result;
         }
@@ -303,7 +303,7 @@ TEST(DesCryptTests, DecryptShortDataTest)
 
             DesCrypt::Key desKey(key.begin(), key.end());
             DesCrypt::DesDecryptor dec(desKey);
-            dec.DecryptBlock(result.begin(), data.begin(), data.end());
+            dec.DecryptBlock(result.begin(), result.end(), data.begin(), data.end());
 
             return result;
         }
@@ -337,7 +337,7 @@ TEST(DesCryptTests, DecryptLongDataTest)
 
             DesCrypt::Key desKey(key.begin(), key.end());
             DesCrypt::DesDecryptor dec(desKey);
-            dec.DecryptBlock(result.begin(), data.begin(), data.end());
+            dec.DecryptBlock(result.begin(), result.end(), data.begin(), data.end());
 
             return result;
         }
@@ -377,123 +377,95 @@ TEST(DesCryptTests, LongKeyTest)
 
 TEST(DesCryptTests, OutIteratorUsageEncryptTest)
 {
-    struct OutputItMock
-    {
-        OutputItMock(size_t & asteriskCalls, size_t & incrementCalls)
-            : AsteriskCalls_(asteriskCalls)
-            , IncrementCalls_(incrementCalls)
-        { }
-
-        uint8_t & operator*()
-        {
-            ++AsteriskCalls_;
-
-            static uint8_t dummy = 0;
-            return dummy;
-        }
-
-        OutputItMock operator++(int)
-        {
-            ++IncrementCalls_;
-
-            return *this;
-        }
-
-        size_t & AsteriskCalls_;
-        size_t & IncrementCalls_;
-    };
-
     {
         std::array<uint8_t, DesCrypt::BlockSize> data = { 0xe5, 0x1a, 0x9f, 0xd4, 0x19, 0xa7, 0x93, 0x44 };
         std::array<uint8_t, 8> key = { 0xda, 0xec, 0x68, 0xae, 0x83, 0xe0, 0x1e, 0xab };
 
-        size_t asteriskCalls = 0;
+        std::array<uint8_t, 11> fact = {};
-        size_t incrementCalls = 0;
+        // First and last 3 bytes should be untouched.
-        OutputItMock it(asteriskCalls, incrementCalls);
+        std::array<uint8_t, 11> expected = { 0x00, 0x00, 0x00, 0x42, 0x27, 0x88, 0xa6, 0x7b, 0x00, 0x00, 0x00 };
 
         DesCrypt::Key desKey(key.begin(), key.end());
         DesCrypt::DesEncryptor enc(desKey);
-        enc.EncryptBlock(it, data.begin(), data.end());
+        enc.EncryptBlock(fact.begin() + 3, fact.end() - 3, data.begin(), data.end());
 
-        ASSERT_EQ(8, asteriskCalls);
+        ASSERT_EQ(expected, fact);
-        ASSERT_EQ(8, incrementCalls);
     }
 
     {
-        std::array<uint8_t, 11> data = { 0xe5, 0x1a, 0x9f, 0xd4, 0x19, 0x9f, 0x9f, 0x9f, 0x9f, 0x9f, 0x9f };
+        std::array<uint8_t, DesCrypt::BlockSize + 2> data = { 0xe5, 0x1a, 0x9f, 0xd4, 0x19, 0xa7, 0x93, 0x44, 0x44, 0x44  };
         std::array<uint8_t, 8> key = { 0xda, 0xec, 0x68, 0xae, 0x83, 0xe0, 0x1e, 0xab };
 
-        size_t asteriskCalls = 0;
+        std::array<uint8_t, 12> fact = {};
-        size_t incrementCalls = 0;
+        // First and last 4 bytes should be untouched.
-        OutputItMock it(asteriskCalls, incrementCalls);
+        std::array<uint8_t, 12> expected = { 0x00, 0x00, 0x00, 0x00, 0x42, 0x27, 0x88, 0xa6, 0x00, 0x00, 0x00, 0x00 };
 
         DesCrypt::Key desKey(key.begin(), key.end());
         DesCrypt::DesEncryptor enc(desKey);
-        enc.EncryptBlock(it, data.begin(), data.end());
+        enc.EncryptBlock(fact.begin() + 4, fact.end() - 4, data.begin(), data.end());
 
-        ASSERT_EQ(8, asteriskCalls);
+        ASSERT_EQ(expected, fact);
-        ASSERT_EQ(8, incrementCalls);
+    }
+
+    {
+        std::array<uint8_t, DesCrypt::BlockSize + 2> data = { 0xe5, 0x1a, 0x9f, 0xd4, 0x19, 0xa7, 0x93, 0x44, 0x44, 0x44  };
+        std::array<uint8_t, 8> key = { 0xda, 0xec, 0x68, 0xae, 0x83, 0xe0, 0x1e, 0xab };
+
+        std::array<uint8_t, 12> fact = {};
+        std::array<uint8_t, 12> expected = {};
+
+        DesCrypt::Key desKey(key.begin(), key.end());
+        DesCrypt::DesEncryptor enc(desKey);
+        enc.EncryptBlock(fact.begin() + 3, fact.begin() + 3, data.begin(), data.end());
+
+        ASSERT_EQ(expected, fact);
     }
 }
 
 TEST(DesCryptTests, OutIteratorUsageDecryptTest)
 {
-    struct OutputItMock
-    {
-        OutputItMock(size_t & asteriskCalls, size_t & incrementCalls)
-            : AsteriskCalls_(asteriskCalls)
-            , IncrementCalls_(incrementCalls)
-        { }
-
-        uint8_t & operator*()
-        {
-            ++AsteriskCalls_;
-
-            static uint8_t dummy = 0;
-            return dummy;
-        }
-
-        OutputItMock operator++(int)
-        {
-            ++IncrementCalls_;
-
-            return *this;
-        }
-
-        size_t & AsteriskCalls_;
-        size_t & IncrementCalls_;
-    };
-
     {
         std::array<uint8_t, DesCrypt::BlockSize> data = { 0xe5, 0x1a, 0x9f, 0xd4, 0x19, 0xa7, 0x93, 0x44 };
         std::array<uint8_t, 8> key = { 0xda, 0xec, 0x68, 0xae, 0x83, 0xe0, 0x1e, 0xab };
 
-        size_t asteriskCalls = 0;
+        std::array<uint8_t, 11> fact = {};
-        size_t incrementCalls = 0;
+        // First and last 3 bytes should be untouched.
-        OutputItMock it(asteriskCalls, incrementCalls);
+        std::array<uint8_t, 11> expected = { 0x00, 0x00, 0x00, 0x45, 0x69, 0x71, 0x17, 0x13, 0x00, 0x00, 0x00 };
 
         DesCrypt::Key desKey(key.begin(), key.end());
         DesCrypt::DesDecryptor dec(desKey);
-        dec.DecryptBlock(it, data.begin(), data.end());
+        dec.DecryptBlock(fact.begin() + 3, fact.end() - 3, data.begin(), data.end());
 
-        ASSERT_EQ(8, asteriskCalls);
+        ASSERT_EQ(expected, fact);
-        ASSERT_EQ(8, incrementCalls);
     }
 
     {
-        std::array<uint8_t, 11> data = { 0xe5, 0x1a, 0x9f, 0xd4, 0x19, 0x9f, 0x9f, 0x9f, 0x9f, 0x9f, 0x9f };
+        std::array<uint8_t, DesCrypt::BlockSize + 2> data = { 0xe5, 0x1a, 0x9f, 0xd4, 0x19, 0xa7, 0x93, 0x44, 0x44, 0x44  };
         std::array<uint8_t, 8> key = { 0xda, 0xec, 0x68, 0xae, 0x83, 0xe0, 0x1e, 0xab };
 
-        size_t asteriskCalls = 0;
+        std::array<uint8_t, 12> fact = {};
-        size_t incrementCalls = 0;
+        // First and last 4 bytes should be untouched.
-        OutputItMock it(asteriskCalls, incrementCalls);
+        std::array<uint8_t, 12> expected = { 0x00, 0x00, 0x00, 0x00, 0x45, 0x69, 0x71, 0x17, 0x00, 0x00, 0x00, 0x00 };
 
         DesCrypt::Key desKey(key.begin(), key.end());
         DesCrypt::DesDecryptor dec(desKey);
-        dec.DecryptBlock(it, data.begin(), data.end());
+        dec.DecryptBlock(fact.begin() + 4, fact.end() - 4, data.begin(), data.end());
 
-        ASSERT_EQ(8, asteriskCalls);
+        ASSERT_EQ(expected, fact);
-        ASSERT_EQ(8, incrementCalls);
+    }
+
+    {
+        std::array<uint8_t, DesCrypt::BlockSize + 2> data = { 0xe5, 0x1a, 0x9f, 0xd4, 0x19, 0xa7, 0x93, 0x44, 0x44, 0x44  };
+        std::array<uint8_t, 8> key = { 0xda, 0xec, 0x68, 0xae, 0x83, 0xe0, 0x1e, 0xab };
+
+        std::array<uint8_t, 12> fact = {};
+        std::array<uint8_t, 12> expected = {};
+
+        DesCrypt::Key desKey(key.begin(), key.end());
+        DesCrypt::DesDecryptor dec(desKey);
+        dec.DecryptBlock(fact.begin() + 3, fact.begin() + 3, data.begin(), data.end());
+
+        ASSERT_EQ(expected, fact);
     }
 }
 
@@ -504,7 +476,7 @@ static std::vector<uint8_t> EncryptThroughBase(const Encryptor<Impl> & enc,
     std::vector<uint8_t> result;
     result.resize(enc.GetBlockSize(), 0);
 
-    enc.EncryptBlock(result.begin(), begin, end);
+    enc.EncryptBlock(result.begin(), result.end(), begin, end);
     return result;
 }
 
@@ -547,7 +519,7 @@ static std::vector<uint8_t> DecryptThroughBase(const Decryptor<Impl> & dec,
     std::vector<uint8_t> result;
     result.resize(dec.GetBlockSize(), 0);
 
-    dec.DecryptBlock(result.begin(), begin, end);
+    dec.DecryptBlock(result.begin(), result.end(), begin, end);
     return result;
 }