Problem: use of malloc is error-prone

Solution: use std::vector instead, simplify control-flow

src/curve_client.cpp

@@ -175,20 +175,17 @@ int zmq::curve_client_t::process_welcome (const uint8_t *msg_data_,
 int zmq::curve_client_t::produce_initiate (msg_t *msg_)
 {
     const size_t metadata_length = basic_properties_len ();
-    unsigned char *metadata_plaintext =
-      static_cast<unsigned char *> (malloc (metadata_length));
-    alloc_assert (metadata_plaintext);
+    std::vector<unsigned char> metadata_plaintext (metadata_length);
 
-    add_basic_properties (metadata_plaintext, metadata_length);
+    add_basic_properties (&metadata_plaintext[0], metadata_length);
 
-    size_t msg_size = 113 + 128 + crypto_box_BOXZEROBYTES + metadata_length;
+    const size_t msg_size =
+      113 + 128 + crypto_box_BOXZEROBYTES + metadata_length;
     int rc = msg_->init_size (msg_size);
     errno_assert (rc == 0);
 
     rc = _tools.produce_initiate (msg_->data (), msg_size, cn_nonce,
-                                  metadata_plaintext, metadata_length);
-
-    free (metadata_plaintext);
+                                  &metadata_plaintext[0], metadata_length);
 
     if (-1 == rc) {
         session->get_socket ()->event_handshake_failed_protocol (
@@ -217,36 +214,30 @@ int zmq::curve_client_t::process_ready (const uint8_t *msg_data_,
     const size_t clen = (msg_size_ - 14) + crypto_box_BOXZEROBYTES;
 
     uint8_t ready_nonce[crypto_box_NONCEBYTES];
-    uint8_t *ready_plaintext =
-      static_cast<uint8_t *> (malloc (crypto_box_ZEROBYTES + clen));
-    alloc_assert (ready_plaintext);
-    uint8_t *ready_box =
-      static_cast<uint8_t *> (malloc (crypto_box_BOXZEROBYTES + 16 + clen));
-    alloc_assert (ready_box);
-
-    memset (ready_box, 0, crypto_box_BOXZEROBYTES);
-    memcpy (ready_box + crypto_box_BOXZEROBYTES, msg_data_ + 14,
+    std::vector<uint8_t> ready_plaintext (crypto_box_ZEROBYTES + clen);
+    std::vector<uint8_t> ready_box (crypto_box_BOXZEROBYTES + 16 + clen);
+
+    std::fill (ready_box.begin (), ready_box.begin () + crypto_box_BOXZEROBYTES,
+               0);
+    memcpy (&ready_box[crypto_box_BOXZEROBYTES], msg_data_ + 14,
             clen - crypto_box_BOXZEROBYTES);
 
     memcpy (ready_nonce, "CurveZMQREADY---", 16);
     memcpy (ready_nonce + 16, msg_data_ + 6, 8);
     cn_peer_nonce = get_uint64 (msg_data_ + 6);
 
-    int rc = crypto_box_open_afternm (ready_plaintext, ready_box, clen,
+    int rc = crypto_box_open_afternm (&ready_plaintext[0], &ready_box[0], clen,
                                       ready_nonce, cn_precom);
-    free (ready_box);
 
     if (rc != 0) {
         session->get_socket ()->event_handshake_failed_protocol (
           session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_CRYPTOGRAPHIC);
         errno = EPROTO;
-        free (ready_plaintext);
         return -1;
     }
 
-    rc = parse_metadata (ready_plaintext + crypto_box_ZEROBYTES,
+    rc = parse_metadata (&ready_plaintext[crypto_box_ZEROBYTES],
                          clen - crypto_box_ZEROBYTES);
-    free (ready_plaintext);
 
     if (rc == 0)
         _state = connected;

src/curve_client_tools.hpp

@@ -47,6 +47,8 @@
 #include "wire.hpp"
 #include "err.hpp"
 
+#include <vector>
+
 namespace zmq
 {
 struct curve_client_tools_t
@@ -164,30 +166,30 @@ struct curve_client_tools_t
             return -1;
 
         uint8_t initiate_nonce[crypto_box_NONCEBYTES];
-        uint8_t *initiate_box = static_cast<uint8_t *> (
-          malloc (crypto_box_BOXZEROBYTES + 144 + metadata_length_));
-        alloc_assert (initiate_box);
-        uint8_t *initiate_plaintext = static_cast<uint8_t *> (
-          malloc (crypto_box_ZEROBYTES + 128 + metadata_length_));
-        alloc_assert (initiate_plaintext);
+        std::vector<uint8_t> initiate_box (crypto_box_BOXZEROBYTES + 144
+                                           + metadata_length_);
+        std::vector<uint8_t> initiate_plaintext (crypto_box_ZEROBYTES + 128
+                                                 + metadata_length_);
 
         //  Create Box [C + vouch + metadata](C'->S')
-        memset (initiate_plaintext, 0, crypto_box_ZEROBYTES);
-        memcpy (initiate_plaintext + crypto_box_ZEROBYTES, public_key_, 32);
-        memcpy (initiate_plaintext + crypto_box_ZEROBYTES + 32, vouch_nonce + 8,
+        std::fill (initiate_plaintext.begin (),
+                   initiate_plaintext.begin () + crypto_box_ZEROBYTES, 0);
+        memcpy (&initiate_plaintext[crypto_box_ZEROBYTES], public_key_, 32);
+        memcpy (&initiate_plaintext[crypto_box_ZEROBYTES + 32], vouch_nonce + 8,
                 16);
-        memcpy (initiate_plaintext + crypto_box_ZEROBYTES + 48,
+        memcpy (&initiate_plaintext[crypto_box_ZEROBYTES + 48],
                 vouch_box + crypto_box_BOXZEROBYTES, 80);
-        memcpy (initiate_plaintext + crypto_box_ZEROBYTES + 48 + 80,
-                metadata_plaintext_, metadata_length_);
+        if (metadata_length_) {
+            memcpy (&initiate_plaintext[crypto_box_ZEROBYTES + 48 + 80],
+                    metadata_plaintext_, metadata_length_);
+        }
 
         memcpy (initiate_nonce, "CurveZMQINITIATE", 16);
         put_uint64 (initiate_nonce + 16, cn_nonce_);
 
-        rc = crypto_box (initiate_box, initiate_plaintext,
+        rc = crypto_box (&initiate_box[0], &initiate_plaintext[0],
                          crypto_box_ZEROBYTES + 128 + metadata_length_,
                          initiate_nonce, cn_server_, cn_secret_);
-        free (initiate_plaintext);
 
         if (rc == -1)
             return -1;
@@ -203,9 +205,8 @@ struct curve_client_tools_t
         //  Short nonce, prefixed by "CurveZMQINITIATE"
         memcpy (initiate + 105, initiate_nonce + 16, 8);
         //  Box [C + vouch + metadata](C'->S')
-        memcpy (initiate + 113, initiate_box + crypto_box_BOXZEROBYTES,
+        memcpy (initiate + 113, &initiate_box[crypto_box_BOXZEROBYTES],
                 128 + metadata_length_ + crypto_box_BOXZEROBYTES);
-        free (initiate_box);
 
         return 0;
     }

src/curve_mechanism_base.cpp

@@ -63,18 +63,17 @@ int zmq::curve_mechanism_base_t::encode (msg_t *msg_)
     if (msg_->flags () & msg_t::command)
         flags |= 0x02;
 
-    uint8_t *message_plaintext = static_cast<uint8_t *> (malloc (mlen));
-    alloc_assert (message_plaintext);
+    std::vector<uint8_t> message_plaintext (mlen);
 
-    memset (message_plaintext, 0, crypto_box_ZEROBYTES);
+    std::fill (message_plaintext.begin (),
+               message_plaintext.begin () + crypto_box_ZEROBYTES, 0);
     message_plaintext[crypto_box_ZEROBYTES] = flags;
-    memcpy (message_plaintext + crypto_box_ZEROBYTES + 1, msg_->data (),
+    memcpy (&message_plaintext[crypto_box_ZEROBYTES + 1], msg_->data (),
             msg_->size ());
 
-    uint8_t *message_box = static_cast<uint8_t *> (malloc (mlen));
-    alloc_assert (message_box);
+    std::vector<uint8_t> message_box (mlen);
 
-    int rc = crypto_box_afternm (message_box, message_plaintext, mlen,
+    int rc = crypto_box_afternm (&message_box[0], &message_plaintext[0], mlen,
                                  message_nonce, cn_precom);
     zmq_assert (rc == 0);
 
@@ -88,12 +87,9 @@ int zmq::curve_mechanism_base_t::encode (msg_t *msg_)
 
     memcpy (message, "\x07MESSAGE", 8);
     memcpy (message + 8, message_nonce + 16, 8);
-    memcpy (message + 16, message_box + crypto_box_BOXZEROBYTES,
+    memcpy (message + 16, &message_box[crypto_box_BOXZEROBYTES],
             mlen - crypto_box_BOXZEROBYTES);
 
-    free (message_plaintext);
-    free (message_box);
-
     cn_nonce++;
 
     return 0;
@@ -137,17 +133,15 @@ int zmq::curve_mechanism_base_t::decode (msg_t *msg_)
 
     const size_t clen = crypto_box_BOXZEROBYTES + msg_->size () - 16;
 
-    uint8_t *message_plaintext = static_cast<uint8_t *> (malloc (clen));
-    alloc_assert (message_plaintext);
-
-    uint8_t *message_box = static_cast<uint8_t *> (malloc (clen));
-    alloc_assert (message_box);
+    std::vector<uint8_t> message_plaintext (clen);
+    std::vector<uint8_t> message_box (clen);
 
-    memset (message_box, 0, crypto_box_BOXZEROBYTES);
-    memcpy (message_box + crypto_box_BOXZEROBYTES, message + 16,
+    std::fill (message_box.begin (),
+               message_box.begin () + crypto_box_BOXZEROBYTES, 0);
+    memcpy (&message_box[crypto_box_BOXZEROBYTES], message + 16,
             msg_->size () - 16);
 
-    rc = crypto_box_open_afternm (message_plaintext, message_box, clen,
+    rc = crypto_box_open_afternm (&message_plaintext[0], &message_box[0], clen,
                                   message_nonce, cn_precom);
     if (rc == 0) {
         rc = msg_->close ();
@@ -162,7 +156,7 @@ int zmq::curve_mechanism_base_t::decode (msg_t *msg_)
         if (flags & 0x02)
             msg_->set_flags (msg_t::command);
 
-        memcpy (msg_->data (), message_plaintext + crypto_box_ZEROBYTES + 1,
+        memcpy (msg_->data (), &message_plaintext[crypto_box_ZEROBYTES + 1],
                 msg_->size ());
     } else {
         // CURVE I : connection key used for MESSAGE is wrong
@@ -170,8 +164,6 @@ int zmq::curve_mechanism_base_t::decode (msg_t *msg_)
           session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_CRYPTOGRAPHIC);
         errno = EPROTO;
     }
-    free (message_plaintext);
-    free (message_box);
 
     return rc;
 }

src/curve_server.cpp

@@ -327,33 +327,29 @@ int zmq::curve_server_t::process_initiate (msg_t *msg_)
     const size_t clen = (size - 113) + crypto_box_BOXZEROBYTES;
 
     uint8_t initiate_nonce[crypto_box_NONCEBYTES];
-    uint8_t *initiate_plaintext =
-      static_cast<uint8_t *> (malloc (crypto_box_ZEROBYTES + clen));
-    alloc_assert (initiate_plaintext);
-    uint8_t *initiate_box =
-      static_cast<uint8_t *> (malloc (crypto_box_BOXZEROBYTES + clen));
-    alloc_assert (initiate_box);
+    std::vector<uint8_t> initiate_plaintext (crypto_box_ZEROBYTES + clen);
+    std::vector<uint8_t> initiate_box (crypto_box_BOXZEROBYTES + clen);
 
     //  Open Box [C + vouch + metadata](C'->S')
-    memset (initiate_box, 0, crypto_box_BOXZEROBYTES);
-    memcpy (initiate_box + crypto_box_BOXZEROBYTES, initiate + 113,
+    std::fill (initiate_box.begin (),
+               initiate_box.begin () + crypto_box_BOXZEROBYTES, 0);
+    memcpy (&initiate_box[crypto_box_BOXZEROBYTES], initiate + 113,
             clen - crypto_box_BOXZEROBYTES);
 
     memcpy (initiate_nonce, "CurveZMQINITIATE", 16);
     memcpy (initiate_nonce + 16, initiate + 105, 8);
     cn_peer_nonce = get_uint64 (initiate + 105);
 
-    const uint8_t *client_key = initiate_plaintext + crypto_box_ZEROBYTES;
+    const uint8_t *client_key = &initiate_plaintext[crypto_box_ZEROBYTES];
 
-    rc = crypto_box_open (initiate_plaintext, initiate_box, clen,
+    rc = crypto_box_open (&initiate_plaintext[0], &initiate_box[0], clen,
                           initiate_nonce, _cn_client, _cn_secret);
     if (rc != 0) {
         // CURVE I: cannot open client INITIATE
         session->get_socket ()->event_handshake_failed_protocol (
           session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_CRYPTOGRAPHIC);
         errno = EPROTO;
-        rc = -1;
-        goto exit;
+        return -1;
     }
 
     uint8_t vouch_nonce[crypto_box_NONCEBYTES];
@@ -363,10 +359,10 @@ int zmq::curve_server_t::process_initiate (msg_t *msg_)
     //  Open Box Box [C',S](C->S') and check contents
     memset (vouch_box, 0, crypto_box_BOXZEROBYTES);
     memcpy (vouch_box + crypto_box_BOXZEROBYTES,
-            initiate_plaintext + crypto_box_ZEROBYTES + 48, 80);
+            &initiate_plaintext[crypto_box_ZEROBYTES + 48], 80);
 
     memcpy (vouch_nonce, "VOUCH---", 8);
-    memcpy (vouch_nonce + 8, initiate_plaintext + crypto_box_ZEROBYTES + 32,
+    memcpy (vouch_nonce + 8, &initiate_plaintext[crypto_box_ZEROBYTES + 32],
             16);
 
     rc = crypto_box_open (vouch_plaintext, vouch_box, sizeof vouch_box,
@@ -376,8 +372,7 @@ int zmq::curve_server_t::process_initiate (msg_t *msg_)
         session->get_socket ()->event_handshake_failed_protocol (
           session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_CRYPTOGRAPHIC);
         errno = EPROTO;
-        rc = -1;
-        goto exit;
+        return -1;
     }
 
     //  What we decrypted must be the client's short-term public key
@@ -389,8 +384,7 @@ int zmq::curve_server_t::process_initiate (msg_t *msg_)
         session->get_socket ()->event_handshake_failed_protocol (
           session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_KEY_EXCHANGE);
         errno = EPROTO;
-        rc = -1;
-        goto exit;
+        return -1;
     }
 
     //  Precompute connection secret from client key
@@ -410,9 +404,8 @@ int zmq::curve_server_t::process_initiate (msg_t *msg_)
             //  reply already, but removing this has some strange side-effect
             //  (probably because the pipe's in_active flag is true until a read
             //  is attempted)
-            rc = receive_and_process_zap_reply ();
-            if (rc == -1)
-                goto exit;
+            if (-1 == receive_and_process_zap_reply ())
+                return -1;
         } else if (!options.zap_enforce_domain) {
             //  This supports the Stonehouse pattern (encryption without
             //  authentication) in legacy mode (domain set but no handler).
@@ -420,21 +413,15 @@ int zmq::curve_server_t::process_initiate (msg_t *msg_)
         } else {
             session->get_socket ()->event_handshake_failed_no_detail (
               session->get_endpoint (), EFAULT);
-            rc = -1;
-            goto exit;
+            return -1;
         }
     } else {
         //  This supports the Stonehouse pattern (encryption without authentication).
         state = sending_ready;
     }
 
-    rc = parse_metadata (initiate_plaintext + crypto_box_ZEROBYTES + 128,
-                         clen - crypto_box_ZEROBYTES - 128);
-
-exit:
-    free (initiate_plaintext);
-    free (initiate_box);
-    return rc;
+    return parse_metadata (&initiate_plaintext[crypto_box_ZEROBYTES + 128],
+                           clen - crypto_box_ZEROBYTES - 128);
 }
 
 int zmq::curve_server_t::produce_ready (msg_t *msg_)
@@ -442,30 +429,27 @@ int zmq::curve_server_t::produce_ready (msg_t *msg_)
     const size_t metadata_length = basic_properties_len ();
     uint8_t ready_nonce[crypto_box_NONCEBYTES];
 
-    uint8_t *ready_plaintext =
-      static_cast<uint8_t *> (malloc (crypto_box_ZEROBYTES + metadata_length));
-    alloc_assert (ready_plaintext);
+    std::vector<uint8_t> ready_plaintext (crypto_box_ZEROBYTES
+                                          + metadata_length);
 
     //  Create Box [metadata](S'->C')
-    memset (ready_plaintext, 0, crypto_box_ZEROBYTES);
-    uint8_t *ptr = ready_plaintext + crypto_box_ZEROBYTES;
+    std::fill (ready_plaintext.begin (),
+               ready_plaintext.begin () + crypto_box_ZEROBYTES, 0);
+    uint8_t *ptr = &ready_plaintext[crypto_box_ZEROBYTES];
 
     ptr += add_basic_properties (ptr, metadata_length);
-    const size_t mlen = ptr - ready_plaintext;
+    const size_t mlen = ptr - &ready_plaintext[0];
 
     memcpy (ready_nonce, "CurveZMQREADY---", 16);
     put_uint64 (ready_nonce + 16, cn_nonce);
 
-    uint8_t *ready_box = static_cast<uint8_t *> (
-      malloc (crypto_box_BOXZEROBYTES + 16 + metadata_length));
-    alloc_assert (ready_box);
+    std::vector<uint8_t> ready_box (crypto_box_BOXZEROBYTES + 16
+                                    + metadata_length);
 
-    int rc = crypto_box_afternm (ready_box, ready_plaintext, mlen, ready_nonce,
-                                 cn_precom);
+    int rc = crypto_box_afternm (&ready_box[0], &ready_plaintext[0], mlen,
+                                 ready_nonce, cn_precom);
     zmq_assert (rc == 0);
 
-    free (ready_plaintext);
-
     rc = msg_->init_size (14 + mlen - crypto_box_BOXZEROBYTES);
     errno_assert (rc == 0);
 
@@ -475,9 +459,8 @@ int zmq::curve_server_t::produce_ready (msg_t *msg_)
     //  Short nonce, prefixed by "CurveZMQREADY---"
     memcpy (ready + 6, ready_nonce + 16, 8);
     //  Box [metadata](S'->C')
-    memcpy (ready + 14, ready_box + crypto_box_BOXZEROBYTES,
+    memcpy (ready + 14, &ready_box[crypto_box_BOXZEROBYTES],
             mlen - crypto_box_BOXZEROBYTES);
-    free (ready_box);
 
     cn_nonce++;