From a4e88fa2610da732ea1125b1ed970baed6d286bb Mon Sep 17 00:00:00 2001 From: lloyd Date: Fri, 15 May 2015 03:31:56 +0000 Subject: Fix various bugs found by Coverity scanner. Uninitialized variables, missing divide by zero checks, missing virtual destructor, etc. Only thing serious is bug in TLS maximum fragment decoder; missing breaks in switch statement meant receiver would treat any negotiated max frament as 4k limit. --- src/cmd/speed.cpp | 2 ++ src/cmd/timer.cpp | 4 ++++ src/cmd/tls_client.cpp | 7 +++++-- src/lib/entropy/unix_procs/unix_procs.cpp | 27 ++++++++++----------------- src/lib/math/mp/mp_misc.cpp | 3 +++ src/lib/math/numbertheory/numthry.cpp | 3 +++ src/lib/math/numbertheory/powm_mnt.cpp | 1 + src/lib/misc/srp6/srp6.h | 2 +- src/lib/stream/rc4/rc4.cpp | 2 +- src/lib/stream/salsa20/salsa20.h | 2 +- src/lib/tls/tls_extensions.cpp | 4 ++++ src/lib/tls/tls_seq_numbers.h | 2 ++ src/lib/tls/tls_session.h | 1 + src/tests/test_rfc6979.cpp | 2 +- 14 files changed, 39 insertions(+), 23 deletions(-) (limited to 'src') diff --git a/src/cmd/speed.cpp b/src/cmd/speed.cpp index 185105ee2..516d89d77 100644 --- a/src/cmd/speed.cpp +++ b/src/cmd/speed.cpp @@ -109,12 +109,14 @@ void report_results(const std::string& algo, std::cout << algo; + const std::ios::fmtflags flags = std::cout.flags(); for(auto i = results.rbegin(); i != results.rend(); ++i) { std::cout << " [" << i->second << "] " << std::fixed << std::setprecision(2) << i->first; } std::cout << std::endl; + std::cout.flags(flags); } void time_transform(std::unique_ptr tf, diff --git a/src/cmd/timer.cpp b/src/cmd/timer.cpp index 369e3fd04..14e55316b 100644 --- a/src/cmd/timer.cpp +++ b/src/cmd/timer.cpp @@ -47,10 +47,14 @@ std::ostream& operator<<(std::ostream& out, Timer& timer) std::string op_or_ops = (timer.events() == 1) ? "op" : "ops"; + const std::ios::fmtflags flags = out.flags(); + out << std::setprecision(2) << std::fixed << timer.ms_per_event() << " ms/op" << " (" << timer.events() << " " << op_or_ops << " in " << timer.milliseconds() << " ms)"; + out.flags(flags); + return out; } diff --git a/src/cmd/tls_client.cpp b/src/cmd/tls_client.cpp index 44238ec64..d96cc47a9 100644 --- a/src/cmd/tls_client.cpp +++ b/src/cmd/tls_client.cpp @@ -91,7 +91,10 @@ bool handshake_complete(const TLS::Session& session) void dgram_socket_write(int sockfd, const byte buf[], size_t length) { - send(sockfd, buf, length, MSG_NOSIGNAL); + int r = send(sockfd, buf, length, MSG_NOSIGNAL); + + if(r == -1) + throw std::runtime_error("Socket write failed errno=" + std::to_string(errno)); } void stream_socket_write(int sockfd, const byte buf[], size_t length) @@ -108,7 +111,7 @@ void stream_socket_write(int sockfd, const byte buf[], size_t length) if(errno == EINTR) sent = 0; else - throw std::runtime_error("Socket::write: Socket write failed"); + throw std::runtime_error("Socket write failed errno=" + std::to_string(errno)); } offset += sent; diff --git a/src/lib/entropy/unix_procs/unix_procs.cpp b/src/lib/entropy/unix_procs/unix_procs.cpp index d9aa787cf..d3208b7fc 100644 --- a/src/lib/entropy/unix_procs/unix_procs.cpp +++ b/src/lib/entropy/unix_procs/unix_procs.cpp @@ -80,24 +80,11 @@ void UnixProcessInfo_EntropySource::poll(Entropy_Accumulator& accum) accum.add(usage, 0.0); } -namespace { - -void do_exec(const std::vector& args) - { - // cleaner way to do this? - const char* arg0 = (args.size() > 0) ? args[0].c_str() : nullptr; - const char* arg1 = (args.size() > 1) ? args[1].c_str() : nullptr; - const char* arg2 = (args.size() > 2) ? args[2].c_str() : nullptr; - const char* arg3 = (args.size() > 3) ? args[3].c_str() : nullptr; - const char* arg4 = (args.size() > 4) ? args[4].c_str() : nullptr; - - ::execl(arg0, arg0, arg1, arg2, arg3, arg4, NULL); - } - -} - void Unix_EntropySource::Unix_Process::spawn(const std::vector& args) { + if(args.empty()) + throw std::invalid_argument("Cannot spawn process without path"); + shutdown(); int pipe[2]; @@ -126,7 +113,13 @@ void Unix_EntropySource::Unix_Process::spawn(const std::vector& arg if(close(STDERR_FILENO) != 0) ::exit(127); - do_exec(args); + const char* arg0 = args[0].c_str(); + const char* arg1 = (args.size() > 1) ? args[1].c_str() : nullptr; + const char* arg2 = (args.size() > 2) ? args[2].c_str() : nullptr; + const char* arg3 = (args.size() > 3) ? args[3].c_str() : nullptr; + const char* arg4 = (args.size() > 4) ? args[4].c_str() : nullptr; + + ::execl(arg0, arg0, arg1, arg2, arg3, arg4, NULL); ::exit(127); } } diff --git a/src/lib/math/mp/mp_misc.cpp b/src/lib/math/mp/mp_misc.cpp index 4f24765bb..3b8be177e 100644 --- a/src/lib/math/mp/mp_misc.cpp +++ b/src/lib/math/mp/mp_misc.cpp @@ -43,6 +43,9 @@ s32bit bigint_cmp(const word x[], size_t x_size, */ word bigint_divop(word n1, word n0, word d) { + if(d == 0) + throw std::runtime_error("bigint_divop divide by zero"); + word high = n1 % d, quotient = 0; for(size_t i = 0; i != MP_WORD_BITS; ++i) diff --git a/src/lib/math/numbertheory/numthry.cpp b/src/lib/math/numbertheory/numthry.cpp index fe943cc6b..900e61724 100644 --- a/src/lib/math/numbertheory/numthry.cpp +++ b/src/lib/math/numbertheory/numthry.cpp @@ -176,6 +176,9 @@ BigInt inverse_mod(const BigInt& n, const BigInt& mod) word monty_inverse(word input) { + if(input == 0) + throw std::runtime_error("monty_inverse: divide by zero"); + word b = input; word x2 = 1, x1 = 0, y2 = 0, y1 = 1; diff --git a/src/lib/math/numbertheory/powm_mnt.cpp b/src/lib/math/numbertheory/powm_mnt.cpp index c8bf0928c..5e797b195 100644 --- a/src/lib/math/numbertheory/powm_mnt.cpp +++ b/src/lib/math/numbertheory/powm_mnt.cpp @@ -137,6 +137,7 @@ Montgomery_Exponentiator::Montgomery_Exponentiator(const BigInt& mod, const BigInt r = BigInt::power_of_2(m_mod_words * BOTAN_MP_WORD_BITS); m_R_mod = r % m_modulus; m_R2_mod = (m_R_mod * m_R_mod) % m_modulus; + m_exp_bits = 0; } } diff --git a/src/lib/misc/srp6/srp6.h b/src/lib/misc/srp6/srp6.h index 3eb21b742..5db433ad6 100644 --- a/src/lib/misc/srp6/srp6.h +++ b/src/lib/misc/srp6/srp6.h @@ -89,7 +89,7 @@ class BOTAN_DLL SRP6_Server_Session private: std::string m_hash_id; BigInt m_B, m_b, m_v, m_S, m_p; - size_t m_p_bytes; + size_t m_p_bytes = 0; }; } diff --git a/src/lib/stream/rc4/rc4.cpp b/src/lib/stream/rc4/rc4.cpp index 096772314..3fd0d2276 100644 --- a/src/lib/stream/rc4/rc4.cpp +++ b/src/lib/stream/rc4/rc4.cpp @@ -114,6 +114,6 @@ void RC4::clear() /* * RC4 Constructor */ -RC4::RC4(size_t s) : SKIP(s) {} +RC4::RC4(size_t s) : SKIP(s), X(0), Y(0) {} } diff --git a/src/lib/stream/salsa20/salsa20.h b/src/lib/stream/salsa20/salsa20.h index d9f67bd24..a2b3790ce 100644 --- a/src/lib/stream/salsa20/salsa20.h +++ b/src/lib/stream/salsa20/salsa20.h @@ -38,7 +38,7 @@ class BOTAN_DLL Salsa20 : public StreamCipher secure_vector m_state; secure_vector m_buffer; - size_t m_position; + size_t m_position = 0; }; } diff --git a/src/lib/tls/tls_extensions.cpp b/src/lib/tls/tls_extensions.cpp index b7ba4a917..5f28c98b8 100644 --- a/src/lib/tls/tls_extensions.cpp +++ b/src/lib/tls/tls_extensions.cpp @@ -246,12 +246,16 @@ Maximum_Fragment_Length::Maximum_Fragment_Length(TLS_Data_Reader& reader, { case 1: m_max_fragment = 512; + break; case 2: m_max_fragment = 1024; + break; case 3: m_max_fragment = 2048; + break; case 4: m_max_fragment = 4096; + break; default: throw TLS_Exception(Alert::ILLEGAL_PARAMETER, "Bad value " + std::to_string(val) + " for max fragment len"); diff --git a/src/lib/tls/tls_seq_numbers.h b/src/lib/tls/tls_seq_numbers.h index 2feef33a9..8ce6ed3be 100644 --- a/src/lib/tls/tls_seq_numbers.h +++ b/src/lib/tls/tls_seq_numbers.h @@ -18,6 +18,8 @@ namespace TLS { class Connection_Sequence_Numbers { public: + virtual ~Connection_Sequence_Numbers() {} + virtual void new_read_cipher_state() = 0; virtual void new_write_cipher_state() = 0; diff --git a/src/lib/tls/tls_session.h b/src/lib/tls/tls_session.h index 31691f078..d7dcc90cb 100644 --- a/src/lib/tls/tls_session.h +++ b/src/lib/tls/tls_session.h @@ -37,6 +37,7 @@ class BOTAN_DLL Session m_ciphersuite(0), m_compression_method(0), m_connection_side(static_cast(0)), + m_srtp_profile(0), m_fragment_size(0) {} diff --git a/src/tests/test_rfc6979.cpp b/src/tests/test_rfc6979.cpp index 4f286b96e..c0bdd5822 100644 --- a/src/tests/test_rfc6979.cpp +++ b/src/tests/test_rfc6979.cpp @@ -48,7 +48,7 @@ size_t rfc6979_testcase(const std::string& q_str, if(gen_0 != exp_k) { std::cout << "RFC 6979 test #" << testcase << " failed; generated k=" - << std::hex << gen_k << " (gen_0)\n"; + << std::hex << gen_0 << " (gen_0)\n"; ++fails; } -- cgit v1.2.3