lib BUGFIX coverity fixes (#532)

* io: handle potential read/write errors

* io: avoid NULL pointer access

* log: free allocated variable argument list

* server_config: only free buffer if it was allocated before

* server_config: evaluate return value of lyd_find_path

* server_config_util_ssh: make hash buffer static to avoid large call stack

* session: protect session status against concurrent access

* session_server: only free attr_p if it was allocated before

* session_server_ssh: make hash buffer static to avoid large call stack

* session_server_tls: remove unneeded pointer resets

---------

Co-authored-by: Michal Struwe <[email protected]>
Co-authored-by: Martin Herberg <[email protected]>

Author: 3 people

src/io.c

@@ -943,6 +943,7 @@ nc_write_msg_io(struct nc_session *session, int io_timeout, int type, ...)
 
         switch (reply->type) {
         case NC_RPL_OK:
+            assert(rpc_envp != NULL);
             if (lyd_new_opaq2(reply_envp, NULL, "ok", NULL, rpc_envp->name.prefix, rpc_envp->name.module_ns, NULL)) {
                 lyd_free_tree(reply_envp);
 

src/log.c

@@ -140,6 +140,7 @@ nc_log_vprintf(const struct nc_session *session, NC_VERB_LEVEL level, const char
 
 cleanup:
     free(msg);
+    va_end(args2);
 }
 
 void

src/server_config.c

@@ -3232,7 +3232,7 @@ nc_server_config_create_cert_to_name(const struct lyd_node *node, struct nc_serv
 
     assert(!strcmp(LYD_NAME(node), "cert-to-name"));
 
-    /* find the list's key */
+    /* find the list's key - ignore result using assert of reference argument instead */
     lyd_find_path(node, "id", 0, &n);
     assert(n);
     id = ((struct lyd_node_term *)n)->value.uint32;

src/server_config_util_ssh.c

@@ -498,11 +498,18 @@ _nc_server_config_add_ssh_user_password(const struct ly_ctx *ctx, const char *tr
     int ret = 0;
     char *hashed_pw = NULL;
     const char *salt = "$6$idsizuippipk$";
-    struct crypt_data cdata = {0};
+    struct crypt_data *cdata = NULL;
 
     NC_CHECK_ARG_RET(NULL, ctx, tree_path, password, config, 1);
 
-    hashed_pw = crypt_r(password, salt, &cdata);
+    cdata = (struct crypt_data *) calloc(sizeof(struct crypt_data), 1);
+    if (cdata == NULL) {
+        ERR(NULL, "Allocation of crypt_data struct failed.");
+        ret = 1;
+        goto cleanup;
+    }
+
+    hashed_pw = crypt_r(password, salt, cdata);
     if (!hashed_pw) {
         ERR(NULL, "Hashing password failed (%s).", strerror(errno));
         ret = 1;
@@ -515,6 +522,7 @@ _nc_server_config_add_ssh_user_password(const struct ly_ctx *ctx, const char *tr
     }
 
 cleanup:
+    free(cdata);
     return ret;
 }
 

src/session.c

@@ -881,8 +881,25 @@ nc_session_free(struct nc_session *session, void (*data_free)(void *))
     struct ly_in *msg;
     struct timespec ts;
     void *p;
+    NC_STATUS status;
 
-    if (!session || (session->status == NC_STATUS_CLOSING)) {
+    if (!session) {
+        return;
+    }
+
+    if ((session->side == NC_SERVER) && (session->flags & NC_SESSION_CALLHOME)) {
+        /* CH LOCK */
+        pthread_mutex_lock(&session->opts.server.ch_lock);
+    }
+
+    status = session->status;
+
+    if ((session->side == NC_SERVER) && (session->flags & NC_SESSION_CALLHOME)) {
+        /* CH UNLOCK */
+        pthread_mutex_unlock(&session->opts.server.ch_lock);
+    }
+
+    if (status == NC_STATUS_CLOSING) {
         return;
     }
 

src/session_server.c

@@ -2766,7 +2766,7 @@ nc_connect_ch_endpt(struct nc_ch_endpt *endpt, nc_server_ch_session_acquire_ctx_
     const struct ly_ctx *ctx = NULL;
     int sock, ret;
     struct timespec ts_cur;
-    char *ip_host;
+    char *ip_host = NULL;
 
     sock = nc_sock_connect(endpt->src_addr, endpt->src_port, endpt->dst_addr, endpt->dst_port,
             NC_CH_CONNECT_TIMEOUT, &endpt->ka, &endpt->sock_pending, &ip_host);

src/session_server_ssh.c

@@ -625,7 +625,8 @@ static int
 nc_server_ssh_compare_password(const char *stored_pw, const char *received_pw)
 {
     char *received_pw_hash = NULL;
-    struct crypt_data cdata = {0};
+    struct crypt_data *cdata;
+    int ret;
 
     NC_CHECK_ARG_RET(NULL, stored_pw, received_pw, 1);
 
@@ -645,13 +646,23 @@ nc_server_ssh_compare_password(const char *stored_pw, const char *received_pw)
         return strcmp(stored_pw + 3, received_pw);
     }
 
-    received_pw_hash = crypt_r(received_pw, stored_pw, &cdata);
+    cdata = (struct crypt_data *) calloc(sizeof(struct crypt_data), 1);
+    if (cdata == NULL) {
+        ERR(NULL, "Allocation of crypt_data struct failed.");
+        return 1;
+    }
+
+    received_pw_hash = crypt_r(received_pw, stored_pw, cdata);
     if (!received_pw_hash) {
         ERR(NULL, "Hashing the password failed (%s).", strerror(errno));
+        free(cdata);
         return 1;
     }
 
-    return strcmp(received_pw_hash, stored_pw);
+    ret = strcmp(received_pw_hash, stored_pw);
+    free(cdata);
+
+    return ret;
 }
 
 API int

src/session_server_tls.c

@@ -331,8 +331,8 @@ static int
 nc_server_tls_cert_to_name(struct nc_ctn *ctn, void *cert_chain, char **username)
 {
     int ret = 1, i, cert_count, fingerprint_match;
-    char *digest_md5 = NULL, *digest_sha1 = NULL, *digest_sha224 = NULL;
-    char *digest_sha256 = NULL, *digest_sha384 = NULL, *digest_sha512 = NULL;
+    char *digest_md5, *digest_sha1, *digest_sha224;
+    char *digest_sha256, *digest_sha384, *digest_sha512;
     void *cert;
 
     /* first make sure the entry is valid */
@@ -372,7 +372,6 @@ nc_server_tls_cert_to_name(struct nc_ctn *ctn, void *cert_chain, char **username
                 fingerprint_match = 1;
             }
             free(digest_md5);
-            digest_md5 = NULL;
 
             /* SHA-1 */
         } else if (!strncmp(ctn->fingerprint, "02", 2)) {
@@ -388,7 +387,6 @@ nc_server_tls_cert_to_name(struct nc_ctn *ctn, void *cert_chain, char **username
                 fingerprint_match = 1;
             }
             free(digest_sha1);
-            digest_sha1 = NULL;
 
             /* SHA-224 */
         } else if (!strncmp(ctn->fingerprint, "03", 2)) {
@@ -404,7 +402,6 @@ nc_server_tls_cert_to_name(struct nc_ctn *ctn, void *cert_chain, char **username
                 fingerprint_match = 1;
             }
             free(digest_sha224);
-            digest_sha224 = NULL;
 
             /* SHA-256 */
         } else if (!strncmp(ctn->fingerprint, "04", 2)) {
@@ -420,7 +417,6 @@ nc_server_tls_cert_to_name(struct nc_ctn *ctn, void *cert_chain, char **username
                 fingerprint_match = 1;
             }
             free(digest_sha256);
-            digest_sha256 = NULL;
 
             /* SHA-384 */
         } else if (!strncmp(ctn->fingerprint, "05", 2)) {
@@ -436,7 +432,6 @@ nc_server_tls_cert_to_name(struct nc_ctn *ctn, void *cert_chain, char **username
                 fingerprint_match = 1;
             }
             free(digest_sha384);
-            digest_sha384 = NULL;
 
             /* SHA-512 */
         } else if (!strncmp(ctn->fingerprint, "06", 2)) {
@@ -452,7 +447,6 @@ nc_server_tls_cert_to_name(struct nc_ctn *ctn, void *cert_chain, char **username
                 fingerprint_match = 1;
             }
             free(digest_sha512);
-            digest_sha512 = NULL;
 
             /* unknown */
         } else {