Fix LGTM warning about overflow checks in SP800-108 KDF

It turns out some KDFs *do* verify that the output length is not
too large, making the KDF truncation bug (#2437) even nastier.

Add comments in KDFs where we are truncating so they can be fixed
later. Also fix SP800-56C which would return the original key length
rather than the possibly truncated key the KDF generated.

1 files changed, 22 insertions, 9 deletions

diff --git a/src/lib/kdf/sp800_108/sp800_108.cpp b/src/lib/kdf/sp800_108/sp800_108.cpp
index fbeae23cd..909e8d47d 100644
--- a/src/lib/kdf/sp800_108/sp800_108.cpp
+++ b/src/lib/kdf/sp800_108/sp800_108.cpp
@@ -18,6 +18,12 @@ size_t SP800_108_Counter::kdf(uint8_t key[], size_t key_len,
                               const uint8_t label[], size_t label_len) const
    {
    const std::size_t prf_len =  m_prf->output_length();
+
+   const uint64_t blocks_required = (key_len + prf_len - 1) / prf_len;
+
+   if(blocks_required > 0xFFFFFFFF)
+      throw Invalid_Argument("SP800_108_Counter output size too large");
+
    const uint8_t delim = 0;
    const uint32_t length = static_cast<uint32_t>(key_len * 8);
 
@@ -29,7 +35,7 @@ size_t SP800_108_Counter::kdf(uint8_t key[], size_t key_len,
    store_be(length, be_len);
    m_prf->set_key(secret, secret_len);
 
-   while(p < key + key_len && counter != 0)
+   while(p < key + key_len)
       {
       const std::size_t to_copy = std::min< std::size_t >(key + key_len - p, prf_len);
       uint8_t be_cnt[4] = { 0 };
@@ -47,8 +53,7 @@ size_t SP800_108_Counter::kdf(uint8_t key[], size_t key_len,
       p += to_copy;
 
       ++counter;
-      if(counter == 0)
-         throw Invalid_Argument("Can't process more than 4GB");
+      BOTAN_ASSERT(counter != 0, "No counter overflow");
       }
 
    return key_len;
@@ -64,6 +69,11 @@ size_t SP800_108_Feedback::kdf(uint8_t key[], size_t key_len,
    const std::size_t iv_len = (salt_len >= prf_len ? prf_len : 0);
    const uint8_t delim = 0;
 
+   const uint64_t blocks_required = (key_len + prf_len - 1) / prf_len;
+
+   if(blocks_required > 0xFFFFFFFF)
+      throw Invalid_Argument("SP800_108_Feedback output size too large");
+
    uint8_t *p = key;
    uint32_t counter = 1;
    uint8_t be_len[4] = { 0 };
@@ -73,7 +83,7 @@ size_t SP800_108_Feedback::kdf(uint8_t key[], size_t key_len,
    store_be(length, be_len);
    m_prf->set_key(secret, secret_len);
 
-   while(p < key + key_len && counter != 0)
+   while(p < key + key_len)
       {
       const std::size_t to_copy = std::min< std::size_t >(key + key_len - p, prf_len);
       uint8_t be_cnt[4] = { 0 };
@@ -93,8 +103,7 @@ size_t SP800_108_Feedback::kdf(uint8_t key[], size_t key_len,
 
       ++counter;
 
-      if(counter == 0)
-         throw Invalid_Argument("Can't process more than 4GB");
+      BOTAN_ASSERT(counter != 0, "No overflow");
       }
 
    return key_len;
@@ -109,6 +118,11 @@ size_t SP800_108_Pipeline::kdf(uint8_t key[], size_t key_len,
    const std::size_t prf_len =  m_prf->output_length();
    const uint8_t delim = 0;
 
+   const uint64_t blocks_required = (key_len + prf_len - 1) / prf_len;
+
+   if(blocks_required > 0xFFFFFFFF)
+      throw Invalid_Argument("SP800_108_Feedback output size too large");
+
    uint8_t *p = key;
    uint32_t counter = 1;
    uint8_t be_len[4] = { 0 };
@@ -123,7 +137,7 @@ size_t SP800_108_Pipeline::kdf(uint8_t key[], size_t key_len,
    std::copy(salt,salt + salt_len,std::back_inserter(ai));
    std::copy(be_len,be_len + 4,std::back_inserter(ai));
 
-   while(p < key + key_len && counter != 0)
+   while(p < key + key_len)
       {
       // A(i)
       m_prf->update(ai);
@@ -148,8 +162,7 @@ size_t SP800_108_Pipeline::kdf(uint8_t key[], size_t key_len,
 
       ++counter;
 
-      if(counter == 0)
-         throw Invalid_Argument("Can't process more than 4GB");
+      BOTAN_ASSERT(counter != 0, "No overflow");
       }
 
    return key_len;