Repository: randombit/botan
Pull request #2363 (2020-06-05 16:52:50)

Fix cppcheck findings
* Variable vs. function shadowing with `CLI::group()`
* Wrong XMSS parameter string to enum mapping
* Duplicate `if` statements in TLS
* Unused variables in PKCS#10 and pubkey tests


Patch:
diff --git a/src/cli/pubkey.cpp b/src/cli/pubkey.cpp
index 39923bc1f..7c7e1bfc0 100644
--- a/src/cli/pubkey.cpp
+++ b/src/cli/pubkey.cpp
@@ -384,19 +384,19 @@ class EC_Group_Info final : public Command
 
       void go() override
          {
-         Botan::EC_Group group(get_arg("name"));
+         Botan::EC_Group ec_group(get_arg("name"));
 
          if(flag_set("pem"))
             {
-            output() << group.PEM_encode();
+            output() << ec_group.PEM_encode();
             }
          else
             {
-            output() << "P = " << std::hex << group.get_p() << "\n"
-                     << "A = " << std::hex << group.get_a() << "\n"
-                     << "B = " << std::hex << group.get_b() << "\n"
-                     << "N = " << std::hex << group.get_order() << "\n"
-                     << "G = " << group.get_g_x() << "," << group.get_g_y() << "\n";
+            output() << "P = " << std::hex << ec_group.get_p() << "\n"
+                     << "A = " << std::hex << ec_group.get_a() << "\n"
+                     << "B = " << std::hex << ec_group.get_b() << "\n"
+                     << "N = " << std::hex << ec_group.get_order() << "\n"
+                     << "G = " << ec_group.get_g_x() << "," << ec_group.get_g_y() << "\n";
             }
 
          }
@@ -425,16 +425,16 @@ class DL_Group_Info final : public Command
 
       void go() override
          {
-         Botan::DL_Group group(get_arg("name"));
+         Botan::DL_Group dl_group(get_arg("name"));
 
          if(flag_set("pem"))
             {
-            output() << group.PEM_encode(Botan::DL_Group::ANSI_X9_42_DH_PARAMETERS);
+            output() << dl_group.PEM_encode(Botan::DL_Group::ANSI_X9_42_DH_PARAMETERS);
             }
          else
             {
-            output() << "P = " << std::hex << group.get_p() << "\n"
-                     << "G = " << group.get_g() << "\n";
+            output() << "P = " << std::hex << dl_group.get_p() << "\n"
+                     << "G = " << dl_group.get_g() << "\n";
             }
 
          }
diff --git a/src/cli/speed.cpp b/src/cli/speed.cpp
index a3e730d80..09ad76d95 100644
--- a/src/cli/speed.cpp
+++ b/src/cli/speed.cpp
@@ -1128,15 +1128,15 @@ class Speed final : public Command
          {
          for(std::string group_name : groups)
             {
-            const Botan::EC_Group group(group_name);
+            const Botan::EC_Group ec_group(group_name);
 
             std::unique_ptr<Timer> add_timer = make_timer(group_name + " add");
             std::unique_ptr<Timer> addf_timer = make_timer(group_name + " addf");
             std::unique_ptr<Timer> dbl_timer = make_timer(group_name + " dbl");
 
-            const Botan::PointGFp& base_point = group.get_base_point();
-            Botan::PointGFp non_affine_pt = group.get_base_point() * 1776; // create a non-affine point
-            Botan::PointGFp pt = group.get_base_point();
+            const Botan::PointGFp& base_point = ec_group.get_base_point();
+            Botan::PointGFp non_affine_pt = ec_group.get_base_point() * 1776; // create a non-affine point
+            Botan::PointGFp pt = ec_group.get_base_point();
 
             std::vector<Botan::BigInt> ws(Botan::PointGFp::WORKSPACE_SIZE);
 
@@ -1173,13 +1173,13 @@ class Speed final : public Command
          {
          for(std::string group_name : groups)
             {
-            const Botan::EC_Group group(group_name);
+            const Botan::EC_Group ec_group(group_name);
 
             std::unique_ptr<Timer> mult_timer = make_timer(group_name + " Montgomery ladder");
             std::unique_ptr<Timer> blinded_mult_timer = make_timer(group_name + " blinded comb");
             std::unique_ptr<Timer> blinded_var_mult_timer = make_timer(group_name + " blinded window");
 
-            const Botan::PointGFp& base_point = group.get_base_point();
+            const Botan::PointGFp& base_point = ec_group.get_base_point();
 
             std::vector<Botan::BigInt> ws;
 
@@ -1187,15 +1187,15 @@ class Speed final : public Command
                   blinded_mult_timer->under(runtime) &&
                   blinded_var_mult_timer->under(runtime))
                {
-               const Botan::BigInt scalar(rng(), group.get_p_bits());
+               const Botan::BigInt scalar(rng(), ec_group.get_p_bits());
 
                const Botan::PointGFp r1 = mult_timer->run([&]() { return base_point * scalar; });
 
                const Botan::PointGFp r2 = blinded_mult_timer->run(
-                  [&]() { return group.blinded_base_point_multiply(scalar, rng(), ws); });
+                  [&]() { return ec_group.blinded_base_point_multiply(scalar, rng(), ws); });
 
                const Botan::PointGFp r3 = blinded_var_mult_timer->run(
-                  [&]() { return group.blinded_var_point_multiply(base_point, scalar, rng(), ws); });
+                  [&]() { return ec_group.blinded_var_point_multiply(base_point, scalar, rng(), ws); });
 
                BOTAN_ASSERT_EQUAL(r1, r2, "Same point computed by Montgomery and comb");
                BOTAN_ASSERT_EQUAL(r1, r3, "Same point computed by Montgomery and window");
@@ -1214,17 +1214,17 @@ class Speed final : public Command
 
          for(std::string group_name : groups)
             {
-            const Botan::EC_Group group(group_name);
+            const Botan::EC_Group ec_group(group_name);
 
             while(uncmp_timer->under(runtime) && cmp_timer->under(runtime))
                {
                const Botan::BigInt k(rng(), 256);
-               const Botan::PointGFp p = group.get_base_point() * k;
+               const Botan::PointGFp p = ec_group.get_base_point() * k;
                const std::vector<uint8_t> os_cmp = p.encode(Botan::PointGFp::COMPRESSED);
                const std::vector<uint8_t> os_uncmp = p.encode(Botan::PointGFp::UNCOMPRESSED);
 
-               uncmp_timer->run([&]() { group.OS2ECP(os_uncmp); });
-               cmp_timer->run([&]() { group.OS2ECP(os_cmp); });
+               uncmp_timer->run([&]() { ec_group.OS2ECP(os_uncmp); });
+               cmp_timer->run([&]() { ec_group.OS2ECP(os_cmp); });
                }
 
             record_result(uncmp_timer);
diff --git a/src/lib/pubkey/xmss/xmss_parameters.cpp b/src/lib/pubkey/xmss/xmss_parameters.cpp
index 0654821fd..55084c8b9 100644
--- a/src/lib/pubkey/xmss/xmss_parameters.cpp
+++ b/src/lib/pubkey/xmss/xmss_parameters.cpp
@@ -32,7 +32,7 @@ XMSS_Parameters::xmss_algorithm_t XMSS_Parameters::xmss_id_from_string(const std
       { return XMSS_SHA2_20_512; }
    if(param_set == "XMSS-SHAKE_10_256")
       { return XMSS_SHAKE_10_256; }
-   if(param_set == "XMSS-SHAKE_10_256")
+   if(param_set == "XMSS-SHAKE_16_256")
       { return XMSS_SHAKE_16_256; }
    if(param_set == "XMSS-SHAKE_20_256")
       { return XMSS_SHAKE_20_256; }
diff --git a/src/lib/tls/tls_channel.cpp b/src/lib/tls/tls_channel.cpp
index 0004bac11..897fed97c 100644
--- a/src/lib/tls/tls_channel.cpp
+++ b/src/lib/tls/tls_channel.cpp
@@ -663,11 +663,13 @@ void Channel::send_alert(const Alert& alert)
       m_pending_state.reset();
 
    if(alert.is_fatal())
+   {
       if(auto active = active_state())
+      {
          m_session_manager.remove_entry(active->server_hello()->session_id());
-
-   if(alert.is_fatal())
+      }
       reset_state();
+   }
 
    if(alert.type() == Alert::CLOSE_NOTIFY || alert.is_fatal())
       {
diff --git a/src/lib/x509/pkcs10.cpp b/src/lib/x509/pkcs10.cpp
index d35e8994a..751097c62 100644
--- a/src/lib/x509/pkcs10.cpp
+++ b/src/lib/x509/pkcs10.cpp
@@ -63,8 +63,6 @@ PKCS10_Request PKCS10_Request::create(const Private_Key& key,
                                       const std::string& padding_scheme,
                                       const std::string& challenge)
    {
-   const std::map<std::string,std::string> sig_opts = { {"padding", padding_scheme} };
-
    AlgorithmIdentifier sig_algo;
    std::unique_ptr<PK_Signer> signer = choose_sig_format(sig_algo, key, rng, hash_fn, padding_scheme);
 


Test patch:
diff --git a/src/tests/test_pubkey.cpp b/src/tests/test_pubkey.cpp
index 6e1f331c7..6c7c21125 100644
--- a/src/tests/test_pubkey.cpp
+++ b/src/tests/test_pubkey.cpp
@@ -398,8 +398,6 @@ PK_Decryption_Test::run_one_test(const std::string& pad_hdr, const VarMap& vars)
 
    std::unique_ptr<Botan::Private_Key> privkey = load_private_key(vars);
 
-   std::vector<std::unique_ptr<Botan::PK_Decryptor>> decryptors;
-
    for(auto const& dec_provider : possible_providers(algo_name()))
       {
       std::unique_ptr<Botan::PK_Decryptor> decryptor;