Merge branch 'tmp-coverity-fixes' into 'master'

Several fixes or annotations attributed to coverity scan

See merge request openconnect/ocserv!237

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;
 }

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;
                 }

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)

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",

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);

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:

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.

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) {