diff --git a/src/auth/gssapi.c b/src/auth/gssapi.c index be32994d..798579eb 100644 --- a/src/auth/gssapi.c +++ b/src/auth/gssapi.c @@ -93,7 +93,7 @@ static void gssapi_vhost_init(void **_vctx, void *pool, void *additional) { int ret; OM_uint32 time, minor; - gss_name_t name = GSS_C_NO_NAME; + const gss_name_t name = GSS_C_NO_NAME; gssapi_cfg_st *config = additional; struct gssapi_vhost_ctx_st *vctx; @@ -136,9 +136,6 @@ static void gssapi_vhost_init(void **_vctx, void *pool, void *additional) } } - if (name != GSS_C_NO_NAME) - gss_release_name(&minor, &name); - *_vctx = vctx; return; } diff --git a/src/auth/pam.c b/src/auth/pam.c index ea2cf688..27b1410f 100644 --- a/src/auth/pam.c +++ b/src/auth/pam.c @@ -61,8 +61,9 @@ enum { static int ocserv_conv(int msg_size, const struct pam_message **msg, struct pam_response **resp, void *uptr) { -struct pam_ctx_st * pctx = uptr; -unsigned i; + struct pam_ctx_st * pctx = uptr; + unsigned i; + int ret; if (msg_size == 0) return PAM_SUCCESS; @@ -79,8 +80,17 @@ unsigned i; case PAM_TEXT_INFO: syslog(LOG_DEBUG, "PAM-auth conv info: %s", msg[i]->msg); - str_append_str(&pctx->msg, msg[i]->msg); - str_append_data(&pctx->msg, " ", 1); + // That should never happen, but also not a big deal if we fail to add message here. + // coverity[check_return : FALSE] + ret = str_append_str(&pctx->msg, msg[i]->msg); + if (ret >= 0) + ret = str_append_data(&pctx->msg, " ", 1); + + if (ret < 0) { + syslog(LOG_ERR, "Error in memory allocation in PAM"); + return PAM_BUF_ERR; + } + pctx->sent_msg = 1; break; case PAM_PROMPT_ECHO_OFF: @@ -93,7 +103,11 @@ unsigned i; } if (msg[i]->msg) { - str_append_str(&pctx->msg, msg[i]->msg); + ret = str_append_str(&pctx->msg, msg[i]->msg); + if (ret < 0) { + syslog(LOG_ERR, "Error in memory allocation in PAM"); + return PAM_BUF_ERR; + } } syslog(LOG_DEBUG, "PAM-auth conv: echo-%s, msg: '%s'", (msg[i]->msg_style==PAM_PROMPT_ECHO_ON)?"on":"off", msg[i]->msg!=NULL?msg[i]->msg:""); @@ -103,8 +117,13 @@ unsigned i; co_resume(); pctx->state = PAM_S_INIT; - if (pctx->password[0] != 0) + if (pctx->password[0] != 0) { pctx->replies[i].resp = strdup(pctx->password); + if (pctx->replies[i].resp == NULL) { + syslog(LOG_ERR, "Error in memory allocation in PAM"); + return PAM_BUF_ERR; + } + } pctx->sent_msg = 0; break; } diff --git a/src/common/common.c b/src/common/common.c index a459d3ee..ce714652 100644 --- a/src/common/common.c +++ b/src/common/common.c @@ -311,18 +311,30 @@ ssize_t force_read_timeout(int sockfd, void *buf, size_t len, unsigned sec) void set_non_block(int fd) { - int val; + int val, ret; val = fcntl(fd, F_GETFL, 0); - fcntl(fd, F_SETFL, val | O_NONBLOCK); + ret = fcntl(fd, F_SETFL, val | O_NONBLOCK); + if (ret == -1) { + /* We log but we do not fail; this seems to be failing when we test + * on 32-bit mode container over a 64-bit kernel. I believe this is related to that: + * https://patchwork.kernel.org/project/qemu-devel/patch/20200331133536.3328-1-linus.walleij@linaro.org/ + */ + int e = errno; + syslog(LOG_ERR, "set_non_block: %s", strerror(e)); + } } void set_block(int fd) { - int val; + int val, ret; val = fcntl(fd, F_GETFL, 0); - fcntl(fd, F_SETFL, val & (~O_NONBLOCK)); + ret = fcntl(fd, F_SETFL, val & (~O_NONBLOCK)); + if (ret == -1) { + int e = errno; + syslog(LOG_ERR, "set_non_block: %s", strerror(e)); + } } ssize_t recv_timeout(int sockfd, void *buf, size_t len, unsigned sec) diff --git a/src/main.c b/src/main.c index cf6d9087..d6c01a5c 100644 --- a/src/main.c +++ b/src/main.c @@ -267,7 +267,12 @@ int _listen_unix_ports(void *pool, struct perm_cfg_st* config, memset(&sa, 0, sizeof(sa)); sa.sun_family = AF_UNIX; strlcpy(sa.sun_path, config->unix_conn_file, sizeof(sa.sun_path)); - remove(sa.sun_path); + if (remove(sa.sun_path) != 0) { + e = errno; + fprintf(stderr, "could not remove unix domain socket['%s']: %s", sa.sun_path, + strerror(e)); + return -1; + } if (config->foreground != 0) fprintf(stderr, "listening (UNIX) on %s...\n", diff --git a/src/occtl/unix.c b/src/occtl/unix.c index 33ad6353..2a481373 100644 --- a/src/occtl/unix.c +++ b/src/occtl/unix.c @@ -1593,8 +1593,13 @@ int handle_events_cmd(struct unix_ctx *ctx, const char *arg, cmd_params_st *para ret = 1; cleanup: talloc_free(data); + // These are indeed dead code but if removed a minor change + // in the code above may result to either memory leak or + // something worse. + // coverity[dead_error_line : FALSE] if (rep1 != NULL) user_list_rep__free_unpacked(rep1, &pa); + // coverity[dead_error_line : FALSE] if (rep2 != NULL) top_update_rep__free_unpacked(rep2, &pa); free_reply(&raw); diff --git a/src/worker-auth.c b/src/worker-auth.c index ecd5bcb6..cbbdc4b2 100644 --- a/src/worker-auth.c +++ b/src/worker-auth.c @@ -1730,10 +1730,11 @@ int post_auth_handler(worker_st * ws, unsigned http_ver) if (sd != -1) close(sd); oclog(ws, LOG_HTTP_DEBUG, "HTTP sending: 401 Unauthorized"); - cstp_printf(ws, + ret = cstp_printf(ws, "HTTP/1.%d 401 %s\r\nContent-Length: 0\r\n\r\n", http_ver, reason); - cstp_fatal_close(ws, GNUTLS_A_ACCESS_DENIED); + if (ret >= 0) + cstp_fatal_close(ws, GNUTLS_A_ACCESS_DENIED); talloc_free(msg); exit_worker(ws); cleanup: diff --git a/src/worker-misc.c b/src/worker-misc.c index f92eecb6..854f1ba7 100644 --- a/src/worker-misc.c +++ b/src/worker-misc.c @@ -125,7 +125,13 @@ int handle_commands_from_main(struct worker_st *ws) if (fd == -1) { oclog(ws, LOG_ERR, "received UDP fd message of wrong type"); - goto udp_fd_fail; + + if (tmsg) + udp_fd_msg__free_unpacked(tmsg, NULL); + + if (DTLS_ACTIVE(ws)->udp_state == UP_WAIT_FD) + DTLS_ACTIVE(ws)->udp_state = UP_DISABLED; + return -1; } set_non_block(fd); @@ -171,13 +177,6 @@ int handle_commands_from_main(struct worker_st *ws) return 0; -udp_fd_fail: - if (tmsg) - udp_fd_msg__free_unpacked(tmsg, NULL); - if (dtls && dtls->dtls_tptr.fd == -1) - dtls->udp_state = UP_DISABLED; - - return -1; } /* Completes the VPN device information. diff --git a/src/worker-vpn.c b/src/worker-vpn.c index cfd080d3..001d8e5a 100644 --- a/src/worker-vpn.c +++ b/src/worker-vpn.c @@ -529,8 +529,13 @@ void send_stats_to_secmod(worker_st * ws, time_t now, unsigned discon_reason) ret = send_msg_to_secmod(ws, sd, CMD_SEC_CLI_STATS, &msg, (pack_size_func)cli_stats_msg__get_packed_size, (pack_func) cli_stats_msg__pack); - if (discon_reason) /* wait for sec-mod to close connection to verify data have been accounted */ - (void)read(sd, buf, sizeof(buf)); + if (discon_reason) { /* wait for sec-mod to close connection to verify data have been accounted */ + e = read(sd, buf, sizeof(buf)); + if (e == -1) { + e = errno; + oclog(ws, LOG_DEBUG, "could not wait for sec-mod: %s\n", strerror(e)); + } + } close(sd); if (ret >= 0) {