From dea5afbfaeba4fdfcf6b5bbf624989d0f875024f Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Mon, 10 Feb 2025 14:48:56 +0100 Subject: [PATCH 1/4] Fix Coverity defect CID 456636: Error handling issues (NEGATIVE_RETURNS) "server_fd" is passed to a parameter that cannot be negative. On error, socket() returns -1, not 0! --- src/http_server.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/http_server.c b/src/http_server.c index 07402691..a1168c8b 100644 --- a/src/http_server.c +++ b/src/http_server.c @@ -231,7 +231,7 @@ int wait_for_http_request(struct vpn_config *config) // Creating socket file descriptor server_fd = socket(AF_INET, SOCK_STREAM, 0); - if (server_fd == 0) { + if (server_fd < 0) { log_error("Failed to create socket\n"); return -1; } From 5f71aa1751753c1c6d3ab0383ed9b1e36ec8b44b Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Mon, 10 Feb 2025 16:07:23 +0100 Subject: [PATCH 2/4] Fix Coverity defect CID 456637: Error handling issues (CHECKED_RETURN) Calling "setsockopt(new_socket, IPPROTO_TCP, 1, &flag, 4U)" without checking return value. This library function may fail and return an error code. --- src/http_server.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/http_server.c b/src/http_server.c index a1168c8b..ad186caa 100644 --- a/src/http_server.c +++ b/src/http_server.c @@ -140,7 +140,10 @@ static int process_request(int new_socket, char *id) int flag = 1; - setsockopt(new_socket, IPPROTO_TCP, TCP_NODELAY, &flag, sizeof(int)); + if (setsockopt(new_socket, IPPROTO_TCP, TCP_NODELAY, &flag, sizeof(flag))) { + log_error("Failed to set socket options\n"); + return -1; + } // Read the request char request[1024]; From 9d026f101265c6a72d4d6557c7e0248e83b192be Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Mon, 10 Feb 2025 16:37:33 +0100 Subject: [PATCH 3/4] Fix Coverity defect CID 456641: (PRINTF_ARGS) Argument "saml_port" to format specifier "%d" was expected to have type "int" but has type "long". --- src/config.h | 2 +- src/http_server.c | 20 ++++++++++---------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/config.h b/src/config.h index 50781e3f..8eecddcf 100644 --- a/src/config.h +++ b/src/config.h @@ -92,7 +92,7 @@ struct vpn_config { int password_set; char otp[OTP_SIZE + 1]; char *cookie; - int saml_port; + uint16_t saml_port; char saml_session_id[MAX_SAML_SESSION_ID_LENGTH + 1]; char *otp_prompt; unsigned int otp_delay; diff --git a/src/http_server.c b/src/http_server.c index ad186caa..f151c4e7 100644 --- a/src/http_server.c +++ b/src/http_server.c @@ -141,7 +141,7 @@ static int process_request(int new_socket, char *id) int flag = 1; if (setsockopt(new_socket, IPPROTO_TCP, TCP_NODELAY, &flag, sizeof(flag))) { - log_error("Failed to set socket options\n"); + log_error("Failed to set socket options: %s\n", strerror(errno)); return -1; } @@ -155,7 +155,7 @@ static int process_request(int new_socket, char *id) // If the received request from the server is larger than the buffer, // the result will not be null-terminated causing strlen to behave wrong. if (read_result < 0) { - log_error("Bad request\n"); + log_error("Bad request: %s\n", strerror(errno)); send_status_response(new_socket, "Invalid redirect response from Fortinet server. VPN could not be established."); return -1; } @@ -230,36 +230,35 @@ int wait_for_http_request(struct vpn_config *config) struct sockaddr_in address; int opt = 1; int addrlen = sizeof(address); - long saml_port = config->saml_port; // Creating socket file descriptor server_fd = socket(AF_INET, SOCK_STREAM, 0); if (server_fd < 0) { - log_error("Failed to create socket\n"); + log_error("Failed to create socket: %s\n", strerror(errno)); return -1; } // Forcefully attaching socket to the port if (setsockopt(server_fd, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(opt))) { close(server_fd); - log_error("Failed to set socket options\n"); + log_error("Failed to set socket options: %s\n", strerror(errno)); return -1; } address.sin_family = AF_INET; address.sin_addr.s_addr = htonl(INADDR_LOOPBACK); - address.sin_port = htons(saml_port); + address.sin_port = htons(config->saml_port); // Forcefully attaching socket to the port if (bind(server_fd, (struct sockaddr *)&address, sizeof(address)) < 0) { close(server_fd); - log_error("Failed to bind socket to port %d\n", saml_port); + log_error("Failed to bind socket to port %u\n", config->saml_port); return -1; } if (listen(server_fd, 3) < 0) { close(server_fd); - log_error("Failed to listen on socket\n"); + log_error("Failed to listen on socket: %s\n", strerror(errno)); return -1; } @@ -267,7 +266,7 @@ int wait_for_http_request(struct vpn_config *config) fd_set readfds; struct timeval tv; - log_info("Listening for SAML login on port %d\n", saml_port); + log_info("Listening for SAML login on port %u\n", config->saml_port); print_url(config); while (max_tries > 0) { @@ -289,7 +288,8 @@ int wait_for_http_request(struct vpn_config *config) (struct sockaddr *)&address, (socklen_t *)&addrlen); if (new_socket < 0) { - log_error("Failed to accept connection\n"); + log_error("Failed to accept connection: %s\n", + strerror(errno)); continue; } } else { From 5e891a046cfb9caf086d9532cb0d61729704aa25 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Mon, 10 Feb 2025 18:14:10 +0100 Subject: [PATCH 4/4] Address compiler warning in CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit warning: ignoring return value of ‘write’ declared with attribute ‘warn_unused_result’ [-Wunused-result] --- src/http_server.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/http_server.c b/src/http_server.c index f151c4e7..b5783467 100644 --- a/src/http_server.c +++ b/src/http_server.c @@ -126,8 +126,10 @@ static void send_status_response(int socket, const char *userMessage) // Using two separate writes here to make the code not more complicated assembling // the buffers. - write(socket, replyHeaderBuffer, strlen(replyHeaderBuffer)); - write(socket, replyBodyBuffer, strlen(replyBodyBuffer)); + if (write(socket, replyHeaderBuffer, strlen(replyHeaderBuffer)) < 0) + log_warn("Failed to write: %s\n", strerror(errno)); + if (write(socket, replyBodyBuffer, strlen(replyBodyBuffer)) < 0) + log_warn("Failed to write: %s\n", strerror(errno)); end: free(replyBodyBuffer);