From e8908d4fb671f9aa014c64c4fe6f3105ac5c4907 Mon Sep 17 00:00:00 2001
From: Jack Lloyd <jack@randombit.net>
Date: Tue, 25 Oct 2016 11:48:25 -0400
Subject: Static analyzer fixes

Check return value of read, found by Clang. See also #677

Remove unused member variable in OpenSSL ECC, found by Clang.

In ECDSA tests, if the pointer is null we should return rather
than dereferencing it. Found by Coverity.
---
 src/tests/test_rng.cpp   | 48 ++++++++++++++++++++++++++++++++----------------
 src/tests/unit_ecdsa.cpp | 11 +++++++----
 2 files changed, 39 insertions(+), 20 deletions(-)

(limited to 'src/tests')

diff --git a/src/tests/test_rng.cpp b/src/tests/test_rng.cpp
index 920afef67..461aaa7c4 100644
--- a/src/tests/test_rng.cpp
+++ b/src/tests/test_rng.cpp
@@ -394,13 +394,13 @@ class HMAC_DRBG_Unit_Tests : public Test
          size_t count = counting_rng.randomize_count();
          Botan::secure_vector<byte> parent_bytes(16), child_bytes(16);
          int fd[2];
-         int rc = pipe(fd);
+         int rc = ::pipe(fd);
          if(rc != 0)
             {
             result.test_failure("failed to create pipe");
             }
 
-         pid_t pid = fork();
+         pid_t pid = ::fork();
          if ( pid == -1 )
             {
             result.test_failure("failed to fork process");
@@ -409,18 +409,34 @@ class HMAC_DRBG_Unit_Tests : public Test
          else if ( pid != 0 )
             {
             // parent process, wait for randomize_count from child's rng
-            close(fd[1]);
-            read(fd[0], &count, sizeof(count));
-            close(fd[0]);
+            ::close(fd[1]);
+            ssize_t got = ::read(fd[0], &count, sizeof(count));
+            ::close(fd[0]);
 
-
-            result.test_eq("parent not reseeded",  counting_rng.randomize_count(), 1);
-            result.test_eq("child reseed occurred", count, 2);
+            if(got > 0)
+               {
+               result.test_eq("expected bytes from child", got, sizeof(count));
+               result.test_eq("parent not reseeded",  counting_rng.randomize_count(), 1);
+               result.test_eq("child reseed occurred", count, 2);
+               }
+            else
+               {
+               result.test_failure("Failed to read count size from child process");
+               }
 
             parent_bytes = rng.random_vec(16);
-            read(fd[0], &child_bytes[0], child_bytes.size());
-            result.test_ne("parent and child output sequences differ", parent_bytes, child_bytes);
-            close(fd[0]);
+            got = ::read(fd[0], &child_bytes[0], child_bytes.size());
+
+            if(got > 0)
+               {
+               result.test_eq("expected bytes from child", got, sizeof(count));
+               result.test_ne("parent and child output sequences differ", parent_bytes, child_bytes);
+               }
+            else
+               {
+               result.test_failure("Failed to read count size from child process");
+               }
+            ::close(fd[0]);
 
             int status = 0;
             ::waitpid(pid, &status, 0);
@@ -428,14 +444,14 @@ class HMAC_DRBG_Unit_Tests : public Test
          else
             {
             // child process, send randomize_count and first output sequence back to parent
-            close(fd[0]);
+            ::close(fd[0]);
             rng.randomize(&child_bytes[0], child_bytes.size());
             count = counting_rng.randomize_count();
-            write(fd[1], &count, sizeof(count));
+            ::write(fd[1], &count, sizeof(count));
             rng.randomize(&child_bytes[0], child_bytes.size());
-            write(fd[1], &child_bytes[0], child_bytes.size());
-            close(fd[1]);
-            _exit(0);
+            ::write(fd[1], &child_bytes[0], child_bytes.size());
+            ::close(fd[1]);
+            ::_exit(0);
             }
 #endif
          return result;
diff --git a/src/tests/unit_ecdsa.cpp b/src/tests/unit_ecdsa.cpp
index 268e5cce0..392b666f5 100644
--- a/src/tests/unit_ecdsa.cpp
+++ b/src/tests/unit_ecdsa.cpp
@@ -212,9 +212,11 @@ Test::Result test_ecdsa_create_save_load()
    Botan::ECDSA_PrivateKey* loaded_ec_key = dynamic_cast<Botan::ECDSA_PrivateKey*>(loaded_key.get());
    result.confirm("the loaded key could be converted into an ECDSA_PrivateKey", loaded_ec_key);
 
-   Botan::PK_Verifier verifier(*loaded_ec_key, "EMSA1(SHA-256)");
-
-   result.confirm("generated signature valid", verifier.verify_message(msg, msg_signature));
+   if(loaded_ec_key)
+      {
+      Botan::PK_Verifier verifier(*loaded_ec_key, "EMSA1(SHA-256)");
+      result.confirm("generated signature valid", verifier.verify_message(msg, msg_signature));
+      }
 
    return result;
    }
@@ -258,7 +260,8 @@ Test::Result test_read_pkcs8()
       std::unique_ptr<Botan::Private_Key> loaded_key_nodp(Botan::PKCS8::load_key(Test::data_file("ecc/nodompar_private.pkcs8.pem"), Test::rng()));
       // anew in each test with unregistered domain-parameters
       Botan::ECDSA_PrivateKey* ecdsa_nodp = dynamic_cast<Botan::ECDSA_PrivateKey*>(loaded_key_nodp.get());
-      result.confirm("key loaded", ecdsa_nodp);
+      if(!ecdsa_nodp)
+         throw Test_Error("Unable to load valid PKCS8 ECDSA key");
 
       Botan::PK_Signer signer(*ecdsa_nodp, Test::rng(), "EMSA1(SHA-256)");
       Botan::PK_Verifier verifier(*ecdsa_nodp, "EMSA1(SHA-256)");
-- 
cgit v1.2.3