Fix multiple security and code quality issues

This commit addresses 13 issues identified in code review:

Critical Issues Fixed:
- Fix NULL pointer dereference in https_resp_cb by checking NULL before use
- Fix NULL pointer dereference in hostname_from_url by validating curl_url_get result
- Fix buffer overflow in addr_list_reduced by validating IP address length
- Fix incorrect fallthrough in https_set_request_version switch statement
- Fix potential integer underflow in dns_poll_cb snprintf calculation
- Add validation for DNS request sizes from network to prevent DoS attacks
- Fix potential memory leak in https_fetch_ctx_init error path
- Fix NULL pointer dereference risk in ring_buffer_free

Medium Priority Issues Fixed:
- Fix signed/unsigned loop logic in dns_server_tcp_respond send loop
- Fix type mismatch in parse_int by using INT_MAX instead of INT32_MAX
- Add portability fallback for accept4 on non-Linux systems
- Fix typo: "listaning" -> "listening" in error message
- Improve documentation for get_io_event return value handling

These fixes improve security, portability, and code robustness.

Author: claude

src/dns_server_tcp.c

@@ -8,6 +8,36 @@
 #include "dns_server_tcp.h"
 #include "logging.h"
 
+// Portability wrapper for accept4
+#ifndef __linux__
+// On non-Linux systems, implement accept4 using accept + fcntl
+static int accept4(int sockfd, struct sockaddr *addr, socklen_t *addrlen, int flags) {
+  int fd = accept(sockfd, addr, addrlen);
+  if (fd == -1) {
+    return -1;
+  }
+  if (flags & SOCK_NONBLOCK) {
+    int fl = fcntl(fd, F_GETFL, 0);
+    if (fl == -1 || fcntl(fd, F_SETFL, fl | O_NONBLOCK) == -1) {
+      int saved_errno = errno;
+      close(fd);
+      errno = saved_errno;
+      return -1;
+    }
+  }
+  if (flags & SOCK_CLOEXEC) {
+    int fl = fcntl(fd, F_GETFD, 0);
+    if (fl == -1 || fcntl(fd, F_SETFD, fl | FD_CLOEXEC) == -1) {
+      int saved_errno = errno;
+      close(fd);
+      errno = saved_errno;
+      return -1;
+    }
+  }
+  return fd;
+}
+#endif
+
 // the following macros require to have client pointer to tcp_client_s structure
 // else: compilation failure will occur
 #define LOG_CLIENT(level, format, args...) LOG(level, "C-%u: " format, client->id, ## args)
@@ -95,6 +125,11 @@ static int get_dns_request(struct tcp_client_s *client,
     char ** dns_req, uint16_t * req_size) {
   // check if whole request is available
   *req_size = ntohs(*((uint16_t*)client->input_buffer));
+  // validate size before using it
+  if (*req_size > DNS_REQUEST_BUFFER_SIZE || *req_size < DNS_HEADER_LENGTH) {
+    WLOG_CLIENT("Invalid DNS request size from network: %u", *req_size);
+    return -1;  // Invalid size
+  }
   uint16_t data_size = sizeof(uint16_t) + *req_size;
   if (data_size > client->input_buffer_used) {
     return 0;  // Partial request
@@ -157,7 +192,8 @@ static void read_cb(struct ev_loop __attribute__((unused)) *loop,
   char *dns_req = NULL;
   uint16_t req_size = 0;
   uint8_t request_received = 0;
-  while (get_dns_request(client, &dns_req, &req_size)) {
+  int result;
+  while ((result = get_dns_request(client, &dns_req, &req_size)) > 0) {
     if (req_size < DNS_HEADER_LENGTH) {
       WLOG_CLIENT("Malformed request received, too short: %u", req_size);
       free(dns_req);
@@ -169,6 +205,13 @@ static void read_cb(struct ev_loop __attribute__((unused)) *loop,
     request_received = 1;
   }
 
+  // Handle error case (invalid size from network)
+  if (result < 0) {
+    WLOG_CLIENT("Invalid DNS request, disconnecting client");
+    remove_client(client);
+    return;
+  }
+
   if (request_received) {
     ev_timer_again(d->loop, &client->timer_watcher);
   }
@@ -257,7 +300,7 @@ static int get_tcp_listen_sock(struct addrinfo *listen_addrinfo) {
   }
 
   if (listen(sock, LISTEN_BACKLOG) == -1) {
-    FLOG("Error listaning on %s:%d TCP: %s (%d)", ipstr, port,
+    FLOG("Error listening on %s:%d TCP: %s (%d)", ipstr, port,
          strerror(errno), errno);
   }
 
@@ -344,8 +387,10 @@ void dns_server_tcp_respond(dns_server_tcp_t *d,
         remove_client(client);
         return;
       }
+      // For EAGAIN/EWOULDBLOCK, don't add to sent, just retry
+    } else {
+      sent += len;
     }
-    sent += len;
 
     if (sent == (ssize_t)resp_len) {
       break;

src/https_client.c

@@ -248,7 +248,7 @@ static void https_set_request_version(https_client_t *client,
   switch (client->opt->use_http_version) {
     case 1:
       http_version_int = CURL_HTTP_VERSION_1_1;
-      __attribute__((fallthrough));
+      break;
     case 2:
       break;
     case 3:
@@ -285,8 +285,7 @@ static void https_fetch_ctx_init(https_client_t *client,
   ctx->cb_data = cb_data;
   ctx->buf = NULL;
   ctx->buflen = 0;
-  ctx->next = client->fetches;
-  client->fetches = ctx;
+  ctx->next = NULL;
 
   ASSERT_CURL_EASY_SETOPT(ctx, CURLOPT_RESOLVE, resolv);
 
@@ -326,12 +325,18 @@ static void https_fetch_ctx_init(https_client_t *client,
   CURLMcode multi_code = curl_multi_add_handle(client->curlm, ctx->curl);
   if (multi_code != CURLM_OK) {
     ELOG_REQ("curl_multi_add_handle error %d: %s", multi_code, curl_multi_strerror(multi_code));
+    // Clean up ctx without trying to remove from multi handle
+    ctx->cb(ctx->cb_data, NULL, 0);
+    curl_easy_cleanup(ctx->curl);
+    free(ctx);
     if (multi_code == CURLM_ABORTED_BY_CALLBACK) {
       WLOG_REQ("Resetting HTTPS client to recover from faulty state!");
       https_client_reset(client);
-    } else {
-      https_fetch_ctx_cleanup(client, NULL, client->fetches, -1);  // dropping current failed request
     }
+  } else {
+    // Only add to linked list if successfully added to multi handle
+    ctx->next = client->fetches;
+    client->fetches = ctx;
   }
 }
 
@@ -590,6 +595,8 @@ static void timer_cb(struct ev_loop __attribute__((unused)) *loop,
   check_multi_info(c);
 }
 
+// Get IO event for a given socket, or find an unused slot (fd == 0)
+// Returns NULL if no matching socket found or no free slot available
 static struct ev_io * get_io_event(struct ev_io io_events[], curl_socket_t sock) {
   for (int i = 0; i < HTTPS_SOCKET_LIMIT; i++) {
     if (io_events[i].fd == sock) {

src/main.c

@@ -58,13 +58,15 @@ static int hostname_from_url(const char* url_in,
     if (rc == CURLUE_OK) {
       char *host = NULL;
       rc = curl_url_get(url, CURLUPART_HOST, &host, 0);
-      const size_t host_len = strlen(host);
-      if (rc == CURLUE_OK && host_len < hostname_len &&
-          host[0] != '[' && host[host_len-1] != ']' && // skip IPv6 address
-          !is_ipv4_address(host)) {
-        strncpy(hostname, host, hostname_len-1);
-        hostname[hostname_len-1] = '\0';
-        res = 1; // success
+      if (rc == CURLUE_OK && host != NULL) {
+        const size_t host_len = strlen(host);
+        if (host_len < hostname_len &&
+            host[0] != '[' && host[host_len-1] != ']' && // skip IPv6 address
+            !is_ipv4_address(host)) {
+          strncpy(hostname, host, hostname_len-1);
+          hostname[hostname_len-1] = '\0';
+          res = 1; // success
+        }
       }
       curl_free(host);
     }
@@ -88,10 +90,10 @@ static void sigpipe_cb(struct ev_loop __attribute__((__unused__)) *loop,
 
 static void https_resp_cb(void *data, char *buf, size_t buflen) {
   request_t *req = (request_t *)data;
-  DLOG("Received response for id: %hX, len: %zu", req->tx_id, buflen);
   if (req == NULL) {
-    FLOG("%04hX: data NULL", req->tx_id);
+    FLOG("https_resp_cb: data NULL");
   }
+  DLOG("Received response for id: %hX, len: %zu", req->tx_id, buflen);
   if (buf != NULL) { // May be NULL for timeout, DNS failure, or something similar.
     if (buflen < DNS_HEADER_LENGTH) {
       WLOG("%04hX: Malformed response received, too short: %u", req->tx_id, buflen);
@@ -182,6 +184,10 @@ static int addr_list_reduced(const char* full_list, const char* list) {
     char current[50];
     const char *comma = strchr(pos, ',');
     size_t ip_len = (size_t)(comma ? comma - pos : end - pos);
+    if (ip_len >= sizeof(current)) {
+      WLOG("IP address too long: %zu bytes", ip_len);
+      return 1;
+    }
     strncpy(current, pos, ip_len);
     current[ip_len] = '\0';
 
@@ -205,10 +211,10 @@ static void dns_poll_cb(const char* hostname, void *data,
   memset(buf, 0, sizeof(buf));
   if (strlen(hostname) > 254) { FLOG("Hostname too long."); }
   int ip_start = snprintf(buf, sizeof(buf) - 1, "%s:443:", hostname);
-  if (ip_start < 0) {
-    abort();  // must be impossible
+  if (ip_start < 0 || ip_start >= (int)(sizeof(buf) - 1)) {
+    FLOG("snprintf failed or buffer too small: %d", ip_start);
   }
-  (void)snprintf(buf + ip_start, sizeof(buf) - 1 - (uint32_t)ip_start, "%s", addr_list);
+  (void)snprintf(buf + ip_start, sizeof(buf) - (size_t)ip_start, "%s", addr_list);
   if (app->resolv == NULL) {
     systemd_notify_ready();
   }

src/options.c

@@ -50,7 +50,7 @@ void options_init(struct Options *opt) {
 int parse_int(char * str) {
   char * endptr = NULL;
   unsigned long int value = strtoul(str, &endptr, 10);
-  if (*endptr != '\0' || value > INT32_MAX) {
+  if (*endptr != '\0' || value > INT_MAX) {
     return -1;
   }
   return (int)value;

src/ring_buffer.c

@@ -35,7 +35,12 @@ void ring_buffer_init(struct ring_buffer **rbp, uint32_t size)
 void ring_buffer_free(struct ring_buffer **rbp)
 {
     struct ring_buffer *rb = *rbp;
+    if (!rb) {
+        return;
+    }
     if (!rb->storage) {
+        free((void*) rb);
+        *rbp = NULL;
         return;
     }
     for (uint32_t i = 0; i < rb->size; i++) {