From 1844fc3284d76a4a788a7489fdb2a5aedde9ccd8 Mon Sep 17 00:00:00 2001 From: Constantin Rack Date: Fri, 7 Nov 2014 16:56:49 +0100 Subject: [PATCH 1/2] Problem: No error-checking of setsockopt ZMQ_CURVE_* z85 keys. Solves #1094. --- src/options.cpp | 52 +++++++++++++++++++++++++++---------------------- 1 file changed, 29 insertions(+), 23 deletions(-) diff --git a/src/options.cpp b/src/options.cpp index 84b2a7b1..5450dbf2 100644 --- a/src/options.cpp +++ b/src/options.cpp @@ -376,19 +376,21 @@ int zmq::options_t::setsockopt (int option_, const void *optval_, } else if (optvallen_ == CURVE_KEYSIZE_Z85 + 1) { - zmq_z85_decode (curve_public_key, (char *) optval_); - mechanism = ZMQ_CURVE; - return 0; + if (zmq_z85_decode (curve_public_key, (char *) optval_)) { + mechanism = ZMQ_CURVE; + return 0; + } } else // Deprecated, not symmetrical with zmq_getsockopt if (optvallen_ == CURVE_KEYSIZE_Z85) { - char z85_key [41]; + char z85_key [CURVE_KEYSIZE_Z85 + 1]; memcpy (z85_key, (char *) optval_, CURVE_KEYSIZE_Z85); z85_key [CURVE_KEYSIZE_Z85] = 0; - zmq_z85_decode (curve_public_key, z85_key); - mechanism = ZMQ_CURVE; - return 0; + if (zmq_z85_decode (curve_public_key, z85_key)) { + mechanism = ZMQ_CURVE; + return 0; + } } break; @@ -400,19 +402,21 @@ int zmq::options_t::setsockopt (int option_, const void *optval_, } else if (optvallen_ == CURVE_KEYSIZE_Z85 + 1) { - zmq_z85_decode (curve_secret_key, (char *) optval_); - mechanism = ZMQ_CURVE; - return 0; + if (zmq_z85_decode (curve_secret_key, (char *) optval_)) { + mechanism = ZMQ_CURVE; + return 0; + } } else // Deprecated, not symmetrical with zmq_getsockopt if (optvallen_ == CURVE_KEYSIZE_Z85) { - char z85_key [41]; + char z85_key [CURVE_KEYSIZE_Z85 + 1]; memcpy (z85_key, (char *) optval_, CURVE_KEYSIZE_Z85); z85_key [CURVE_KEYSIZE_Z85] = 0; - zmq_z85_decode (curve_secret_key, z85_key); - mechanism = ZMQ_CURVE; - return 0; + if (zmq_z85_decode (curve_secret_key, z85_key)) { + mechanism = ZMQ_CURVE; + return 0; + } } break; @@ -425,21 +429,23 @@ int zmq::options_t::setsockopt (int option_, const void *optval_, } else if (optvallen_ == CURVE_KEYSIZE_Z85 + 1) { - zmq_z85_decode (curve_server_key, (char *) optval_); - mechanism = ZMQ_CURVE; - as_server = 0; - return 0; + if (zmq_z85_decode (curve_server_key, (char *) optval_)) { + mechanism = ZMQ_CURVE; + as_server = 0; + return 0; + } } else // Deprecated, not symmetrical with zmq_getsockopt if (optvallen_ == CURVE_KEYSIZE_Z85) { - char z85_key [41]; + char z85_key [CURVE_KEYSIZE_Z85 + 1]; memcpy (z85_key, (char *) optval_, CURVE_KEYSIZE_Z85); z85_key [CURVE_KEYSIZE_Z85] = 0; - zmq_z85_decode (curve_server_key, z85_key); - mechanism = ZMQ_CURVE; - as_server = 0; - return 0; + if (zmq_z85_decode (curve_server_key, z85_key)) { + mechanism = ZMQ_CURVE; + as_server = 0; + return 0; + } } break; # endif From e00ea532df2a8957150e5e37f4886d7abe049af1 Mon Sep 17 00:00:00 2001 From: Constantin Rack Date: Fri, 7 Nov 2014 17:35:41 +0100 Subject: [PATCH 2/2] Add tests for issue #1094. --- tests/test_security_curve.cpp | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/tests/test_security_curve.cpp b/tests/test_security_curve.cpp index af3925ab..75ca8466 100644 --- a/tests/test_security_curve.cpp +++ b/tests/test_security_curve.cpp @@ -217,7 +217,22 @@ int main (void) assert (rc == 0); expect_bounce_fail (server, client); close_zero_linger (client); - + + // Check return codes for invalid buffer sizes + client = zmq_socket (ctx, ZMQ_DEALER); + assert (client); + errno = 0; + rc = zmq_setsockopt (client, ZMQ_CURVE_SERVERKEY, server_public, 123); + assert (rc == -1 && errno == EINVAL); + errno = 0; + rc = zmq_setsockopt (client, ZMQ_CURVE_PUBLICKEY, client_public, 123); + assert (rc == -1 && errno == EINVAL); + errno = 0; + rc = zmq_setsockopt (client, ZMQ_CURVE_SECRETKEY, client_secret, 123); + assert (rc == -1 && errno == EINVAL); + rc = zmq_close (client); + assert (rc == 0); + // Shutdown rc = zmq_close (server); assert (rc == 0);