Fix some warnings from PVS-Studio

No real bugs, but pointed out some odd constructs and duplicated logic

11 files changed, 23 insertions, 14 deletions

diff --git a/src/lib/asn1/asn1_time.cpp b/src/lib/asn1/asn1_time.cpp
index 8de42dfde..63b27c9d7 100644
--- a/src/lib/asn1/asn1_time.cpp
+++ b/src/lib/asn1/asn1_time.cpp
@@ -248,7 +248,7 @@ bool X509_Time::passes_sanity_check() const
 
       http://www.itu.int/ITU-T/studygroups/com17/languages/
       */
-      if (m_hour > 23 || m_minute > 59 || m_second > 59)
+      if(m_second > 59)
          {
          return false;
          }
diff --git a/src/lib/block/aria/aria.cpp b/src/lib/block/aria/aria.cpp
index e92580361..2a02330c9 100644
--- a/src/lib/block/aria/aria.cpp
+++ b/src/lib/block/aria/aria.cpp
@@ -280,12 +280,13 @@ void transform(const uint8_t in[], uint8_t out[], size_t blocks,
    }
 
 // n-bit right shift of Y XORed to X
-template <unsigned int N>
+template<size_t N>
 inline void ARIA_ROL128(const uint32_t X[4], const uint32_t Y[4], uint32_t KS[4])
    {
    // MSVC is not generating a "rotate immediate". Constify to help it along.
-   static const unsigned int Q = 4 - (N / 32);
-   static const unsigned int R = N % 32;
+   static const size_t Q = 4 - (N / 32);
+   static const size_t R = N % 32;
+   static_assert(R > 0 && R < 32, "Rotation in range for type");
    KS[0] = (X[0]) ^ ((Y[(Q  )%4])>>R) ^ ((Y[(Q+3)%4])<<(32-R));
    KS[1] = (X[1]) ^ ((Y[(Q+1)%4])>>R) ^ ((Y[(Q  )%4])<<(32-R));
    KS[2] = (X[2]) ^ ((Y[(Q+2)%4])>>R) ^ ((Y[(Q+1)%4])<<(32-R));
diff --git a/src/lib/math/numbertheory/make_prm.cpp b/src/lib/math/numbertheory/make_prm.cpp
index d18c82c0b..7455a0049 100644
--- a/src/lib/math/numbertheory/make_prm.cpp
+++ b/src/lib/math/numbertheory/make_prm.cpp
@@ -112,8 +112,11 @@ BigInt random_prime(RandomNumberGenerator& rng,
       {
       for(;;)
          {
-         size_t idx = make_uint16(rng.next_byte(), rng.next_byte()) % PRIME_TABLE_SIZE;
-         uint16_t small_prime = PRIMES[idx];
+         // This is slightly biased, but for small primes it does not seem to matter
+         const uint8_t b0 = rng.next_byte();
+         const uint8_t b1 = rng.next_byte();
+         const size_t idx = make_uint16(b0, b1) % PRIME_TABLE_SIZE;
+         const uint16_t small_prime = PRIMES[idx];
 
          if(high_bit(small_prime) == bits)
             return small_prime;
diff --git a/src/lib/misc/nist_keywrap/nist_keywrap.cpp b/src/lib/misc/nist_keywrap/nist_keywrap.cpp
index c349f97aa..3c673696d 100644
--- a/src/lib/misc/nist_keywrap/nist_keywrap.cpp
+++ b/src/lib/misc/nist_keywrap/nist_keywrap.cpp
@@ -190,7 +190,7 @@ nist_key_unwrap_padded(const uint8_t input[],
 
    const size_t len = (ICV_out & 0xFFFFFFFF);
 
-   if(len > R.size() || len < R.size() - 8)
+   if(R.size() < 8 || len > R.size() || len < R.size() - 8)
       throw Integrity_Failure("NIST key unwrap failed");
 
    const size_t padding = R.size() - len;
diff --git a/src/lib/pubkey/ed25519/ed25519_key.cpp b/src/lib/pubkey/ed25519/ed25519_key.cpp
index bdb6a8547..ee1d19edb 100644
--- a/src/lib/pubkey/ed25519/ed25519_key.cpp
+++ b/src/lib/pubkey/ed25519/ed25519_key.cpp
@@ -47,7 +47,7 @@ Ed25519_PrivateKey::Ed25519_PrivateKey(const secure_vector<uint8_t>& secret_key)
    if(secret_key.size() == 64)
       {
       m_private = secret_key;
-      m_public.assign(&m_private[32], &m_private[64]);
+      m_public.assign(m_private.begin() + 32, m_private.end());
       }
    else if(secret_key.size() == 32)
       {
diff --git a/src/lib/pubkey/mce/goppa_code.cpp b/src/lib/pubkey/mce/goppa_code.cpp
index 3cb92aaca..8183dd1d4 100644
--- a/src/lib/pubkey/mce/goppa_code.cpp
+++ b/src/lib/pubkey/mce/goppa_code.cpp
@@ -114,6 +114,7 @@ secure_vector<gf2m> goppa_decode(const polyn_gf2m & syndrom_polyn,
 
       gf2m tmp;
       tmp = gray_to_lex(current);
+      /// XXX double assignment, possible bug?
       if(tmp >= code_length) /* invalid root */
          {
          result[i] = i;
diff --git a/src/lib/pubkey/mce/polyn_gf2m.cpp b/src/lib/pubkey/mce/polyn_gf2m.cpp
index 2541c164f..c08870360 100644
--- a/src/lib/pubkey/mce/polyn_gf2m.cpp
+++ b/src/lib/pubkey/mce/polyn_gf2m.cpp
@@ -124,7 +124,7 @@ void polyn_gf2m::realloc(uint32_t new_size)
    }
 
 polyn_gf2m::polyn_gf2m(const uint8_t* mem, uint32_t mem_len, std::shared_ptr<GF2m_Field> sp_field)
-   :msp_field(sp_field)
+   :msp_field(sp_field), m_deg(-1)
    {
    if(mem_len % sizeof(gf2m))
       {
diff --git a/src/lib/pubkey/mceies/mceies.cpp b/src/lib/pubkey/mceies/mceies.cpp
index 875c9dd10..3f8562e17 100644
--- a/src/lib/pubkey/mceies/mceies.cpp
+++ b/src/lib/pubkey/mceies/mceies.cpp
@@ -23,6 +23,7 @@ secure_vector<uint8_t> aead_key(const secure_vector<uint8_t>& mk,
       return mk;
 
    secure_vector<uint8_t> r(aead.key_spec().maximum_keylength());
+   BOTAN_ASSERT_NOMSG(r.size() > 0);
    for(size_t i = 0; i != mk.size(); ++i)
       r[i % r.size()] ^= mk[i];
    return r;
diff --git a/src/lib/pubkey/xmss/xmss_privatekey.cpp b/src/lib/pubkey/xmss/xmss_privatekey.cpp
index 426ebeb61..05d61981e 100644
--- a/src/lib/pubkey/xmss/xmss_privatekey.cpp
+++ b/src/lib/pubkey/xmss/xmss_privatekey.cpp
@@ -112,9 +112,9 @@ XMSS_PrivateKey::tree_hash(size_t start_idx,
 #if defined(BOTAN_TARGET_OS_HAS_THREADS)
       }
 
-   size_t subtrees = 1 << split_level;
-   size_t last_idx = static_cast<size_t>(1 << (target_node_height)) + start_idx;
-   size_t offs = (last_idx - start_idx) / subtrees;
+   const size_t subtrees = static_cast<size_t>(1) << split_level;
+   const size_t last_idx = (static_cast<size_t>(1) << (target_node_height)) + start_idx;
+   const size_t offs = (last_idx - start_idx) / subtrees;
    uint8_t level = split_level; // current level in the tree
 
    BOTAN_ASSERT((last_idx - start_idx) % subtrees == 0,
@@ -230,7 +230,7 @@ XMSS_PrivateKey::tree_hash_subtree(secure_vector<uint8_t>& result,
 
    uint8_t level = 0; // current level on the node stack.
    XMSS_WOTS_PublicKey pk(m_wots_priv_key.wots_parameters().oid(), seed);
-   size_t last_idx = static_cast<size_t>(1 << target_node_height) + start_idx;
+   const size_t last_idx = (static_cast<size_t>(1) << target_node_height) + start_idx;
 
    for(size_t i = start_idx; i < last_idx; i++)
       {
diff --git a/src/lib/pubkey/xmss/xmss_wots_parameters.cpp b/src/lib/pubkey/xmss/xmss_wots_parameters.cpp
index c2a324475..e5e334bf9 100644
--- a/src/lib/pubkey/xmss/xmss_wots_parameters.cpp
+++ b/src/lib/pubkey/xmss/xmss_wots_parameters.cpp
@@ -79,7 +79,7 @@ XMSS_WOTS_Parameters::XMSS_WOTS_Parameters(ots_algorithm_t oid)
          break;
       }
 
-   m_w == 16 ? m_lg_w = 4 : m_lg_w = 2;
+   m_lg_w = (m_w == 16) ? 4 : 2;
    m_len_1 = static_cast<size_t>(std::ceil((8 * element_size()) / m_lg_w));
    m_len_2 = static_cast<size_t>(
                 floor(log2(m_len_1 * (wots_parameter() - 1)) / m_lg_w) + 1);
diff --git a/src/lib/tls/msg_cert_verify.cpp b/src/lib/tls/msg_cert_verify.cpp
index 8ca01043a..230474e7a 100644
--- a/src/lib/tls/msg_cert_verify.cpp
+++ b/src/lib/tls/msg_cert_verify.cpp
@@ -67,6 +67,9 @@ std::vector<uint8_t> Certificate_Verify::serialize() const
       buf.push_back(get_byte(1, scheme_code));
       }
 
+   if(m_signature.size() > 0xFFFF)
+      throw Encoding_Error("Certificate_Verify signature too long to encode");
+
    const uint16_t sig_len = static_cast<uint16_t>(m_signature.size());
    buf.push_back(get_byte(0, sig_len));
    buf.push_back(get_byte(1, sig_len));