Merge beeb94561e into 79cd21f667

The sprintf function is used without bounds checking to write formatted strings into fixed-size buffers

HISTORY

@@ -1,6 +1,7 @@
 
-Version 1.86  2026-06-25
+Version 1.86  2026-06-26
   * add function fc_parse_version
+  * change return type of format_ip_address and format_ip_port
 
 Version 1.85  2026-06-23
   * add functions fc_safe_srand and fc_safe_rand for more security under Linux

src/multi_socket_client.c

@@ -140,26 +140,27 @@ static int fast_multi_sock_client_do_send(FastMultiSockClient *client,
             if (errno == EAGAIN || errno == EWOULDBLOCK) {
                 break;
             } else if (errno == EINTR) {  //should retry
+                format_ip_address(entry->conn->ip_addr, formatted_ip);
                 logDebug("file: "__FILE__", line: %d, "
                         "server: %s:%u, ignore interupt signal", __LINE__,
-                        format_ip_address(entry->conn->ip_addr, formatted_ip),
-                        entry->conn->port);
+                        formatted_ip, entry->conn->port);
                 continue;
             } else {
                 result = errno != 0 ? errno : ECONNRESET;
+                format_ip_address(entry->conn->ip_addr, formatted_ip);
                 logError("file: "__FILE__", line: %d, "
                         "send to server %s:%u fail, "
                         "errno: %d, error info: %s", __LINE__,
-                        format_ip_address(entry->conn->ip_addr, formatted_ip),
-                        entry->conn->port, result, strerror(result));
+                        formatted_ip, entry->conn->port,
+                        result, strerror(result));
 
                 break;
             }
         } else if (bytes == 0) {
+            format_ip_address(entry->conn->ip_addr, formatted_ip);
             logError("file: "__FILE__", line: %d, "
                     "send to server %s:%u, sock: %d fail, "
-                    "connection disconnected", __LINE__,
-                    format_ip_address(entry->conn->ip_addr, formatted_ip),
+                    "connection disconnected", __LINE__, formatted_ip,
                     entry->conn->port, entry->conn->sock);
 
             result = ECONNRESET;
@@ -203,10 +204,10 @@ static int fast_multi_sock_client_send_data(FastMultiSockClient *client,
         if (client->entries[i].conn->sock < 0) {
             client->entries[i].error_no = ENOTCONN;
             client->entries[i].done = true;
+            format_ip_address(client->entries[i].conn->ip_addr, formatted_ip);
             logError("file: "__FILE__", line: %d, "
                     "NOT connected to %s:%u", __LINE__,
-                    format_ip_address(client->entries[i].conn->ip_addr,
-                        formatted_ip), client->entries[i].conn->port);
+                    formatted_ip, client->entries[i].conn->port);
             continue;
         }
 
@@ -256,26 +257,27 @@ static int fast_multi_sock_client_do_recv(FastMultiSockClient *client,
             if (errno == EAGAIN || errno == EWOULDBLOCK) {
                 break;
             } else if (errno == EINTR) {  //should retry
+                format_ip_address(entry->conn->ip_addr, formatted_ip);
                 logDebug("file: "__FILE__", line: %d, "
                         "server: %s:%u, ignore interupt signal", __LINE__,
-                        format_ip_address(entry->conn->ip_addr, formatted_ip),
-                        entry->conn->port);
+                        formatted_ip, entry->conn->port);
                 continue;
             } else {
                 result = errno != 0 ? errno : ECONNRESET;
+                format_ip_address(entry->conn->ip_addr, formatted_ip);
                 logError("file: "__FILE__", line: %d, "
                         "server: %s:%u, recv failed, "
                         "errno: %d, error info: %s", __LINE__,
-                        format_ip_address(entry->conn->ip_addr, formatted_ip),
-                        entry->conn->port, result, strerror(result));
+                        formatted_ip, entry->conn->port,
+                        result, strerror(result));
 
                 break;
             }
         } else if (bytes == 0) {
+            format_ip_address(entry->conn->ip_addr, formatted_ip);
             logError("file: "__FILE__", line: %d, "
                     "server: %s:%u, sock: %d, recv failed, "
-                    "connection disconnected", __LINE__,
-                    format_ip_address(entry->conn->ip_addr, formatted_ip),
+                    "connection disconnected", __LINE__, formatted_ip,
                     entry->conn->port, entry->conn->sock);
 
             result = ECONNRESET;
@@ -290,10 +292,10 @@ static int fast_multi_sock_client_do_recv(FastMultiSockClient *client,
             entry->recv_stage = fms_stage_recv_body;
             body_length = client->get_body_length_func(&entry->recv_buffer);
             if (body_length < 0) {
+                format_ip_address(entry->conn->ip_addr, formatted_ip);
                 logError("file: "__FILE__", line: %d, "
                         "server: %s:%u, body_length: %d < 0", __LINE__,
-                        format_ip_address(entry->conn->ip_addr, formatted_ip),
-                        entry->conn->port, body_length);
+                        formatted_ip, entry->conn->port, body_length);
                 result = EPIPE;
                 break;
             } else if (body_length == 0) {
@@ -374,10 +376,10 @@ static int fast_multi_sock_client_deal_io(FastMultiSockClient *client)
                     &client->ioevent, index);
 
             if (event & IOEVENT_ERROR) {
+                format_ip_address(entry->conn->ip_addr, formatted_ip);
                 logError("file: "__FILE__", line: %d, "
-                        "server: %s:%u, recv error event: %d, connection "
-                        "reset", __LINE__, format_ip_address(entry->conn->
-                            ip_addr, formatted_ip), entry->conn->port, event);
+                        "server: %s:%u, recv error event: %d, connection reset",
+                        __LINE__, formatted_ip, entry->conn->port, event);
 
                 fast_multi_sock_client_finish(client, entry, ECONNRESET);
                 continue;
@@ -404,10 +406,10 @@ static int fast_multi_sock_client_deal_io(FastMultiSockClient *client)
             if (!client->entries[i].done) {
                 fast_multi_sock_client_finish(client,
                         client->entries + i, ETIMEDOUT);
+                format_ip_address(client->entries[i].conn->ip_addr, formatted_ip);
                 logError("file: "__FILE__", line: %d, "
                         "recv from %s:%u timedout", __LINE__,
-                        format_ip_address(client->entries[i].conn->ip_addr,
-                            formatted_ip), client->entries[i].conn->port);
+                        formatted_ip, client->entries[i].conn->port);
             }
         }
     }

src/server_id_func.c

@@ -259,11 +259,11 @@ static int fc_server_check_ip_port(FCServerConfig *ctx,
                 id1 = current->server->id;
                 id2 = previous->server->id;
             }
+            format_ip_address(previous->ip_addr.str, formatted_ip);
             logError("file: "__FILE__", line: %d, "
                     "config file: %s, duplicate ip:port %s:%u, "
-                    "the server ids: %d, %d", __LINE__,
-                    config_filename, format_ip_address(previous->ip_addr.str,
-                        formatted_ip), previous->port, id1, id2);
+                    "the server ids: %d, %d", __LINE__, config_filename,
+                    formatted_ip, previous->port, id1, id2);
             return EEXIST;
         }
 
@@ -714,14 +714,14 @@ static int check_addresses_duplicate(FCServerConfig *ctx,
     ppend = group_addr->address_array.addrs + group_addr->address_array.count;
     for (ppaddr=group_addr->address_array.addrs+1; ppaddr<ppend; ppaddr++) {
         if (fc_server_cmp_address_ptr(ppaddr, pprevious) == 0) {
+            format_ip_address((*ppaddr)->conn.ip_addr, formatted_ip),
             logError("file: "__FILE__", line: %d, "
                     "config filename: %s, section: %s, group: %.*s, "
                     "duplicate ip and port: %s:%u", __LINE__,
                     config_filename, section_name,
                     group_addr->server_group->group_name.len,
                     group_addr->server_group->group_name.str,
-                    format_ip_address((*ppaddr)->conn.ip_addr, formatted_ip),
-                    (*ppaddr)->conn.port);
+                    formatted_ip, (*ppaddr)->conn.port);
             return EEXIST;
         }
         pprevious = ppaddr;
@@ -967,12 +967,12 @@ static int fc_server_set_host(FCServerConfig *ctx, FCServerInfo *server,
     }
 
     if (!ctx->share_between_groups && (count > 1 && addr->conn.port > 0)) {
+        format_ip_address(addr->conn.ip_addr, formatted_ip),
         logError("file: "__FILE__", line: %d, "
                 "config filename: %s, section: %s, "
                 "host %s:%u belongs to %d groups",
                 __LINE__, config_filename, section_name,
-                format_ip_address(addr->conn.ip_addr, formatted_ip),
-                addr->conn.port, count);
+                formatted_ip, addr->conn.port, count);
         return EEXIST;
     }
 
@@ -1544,9 +1544,10 @@ static void fc_group_servers_to_string(FCServerConfig *ctx,
                     gaddr->server_group->group_name.str,
                     SERVER_ITEM_HOST_AFFIX_STR);
         }
+
+        format_ip_address((*addr)->conn.ip_addr, formatted_ip),
         fast_buffer_append(buffer, " = %s:%u\n",
-                format_ip_address((*addr)->conn.ip_addr, formatted_ip),
-                (*addr)->conn.port);
+                formatted_ip, (*addr)->conn.port);
     }
 }
 
@@ -1655,9 +1656,9 @@ static void fc_server_log_group_servers(FCGroupAddresses *gaddr)
 
     end = gaddr->address_array.addrs + gaddr->address_array.count;
     for (addr=gaddr->address_array.addrs; addr<end; addr++) {
+        format_ip_address((*addr)->conn.ip_addr, formatted_ip),
         logInfo("    %d. %s:%u", (int)(addr - gaddr->address_array.addrs + 1),
-                format_ip_address((*addr)->conn.ip_addr, formatted_ip),
-                (*addr)->conn.port);
+                formatted_ip, (*addr)->conn.port);
     }
 }
 

src/sockopt.h

@@ -706,11 +706,11 @@ static inline bool is_ipv6_addr(const char *ip)
     return (*ip == ':' || strchr(ip, ':') != NULL); //ipv6
 }
 
-static inline const char *format_ip_address(const char *ip, char *buff)
+static inline int format_ip_address(const char *ip, char *buff)
 {
+    int ip_len;
     if (is_ipv6_addr(ip))
     {
-        int ip_len;
         char *p;
 
         ip_len = strlen(ip);
@@ -720,17 +720,17 @@ static inline const char *format_ip_address(const char *ip, char *buff)
         p += ip_len;
         *p++ = ']';
         *p = '\0';
+        return p - buff;
     }
     else
     {
-        strcpy(buff, ip);
+        ip_len = strlen(ip);
+        memcpy(buff, ip, ip_len + 1);
+        return ip_len;
     }
-
-    return buff;
 }
 
-static inline const char *format_ip_port(const char *ip,
-        const int port, char *buff)
+static inline int format_ip_port(const char *ip, const int port, char *buff)
 {
     int ip_len;
     bool is_ipv6;
@@ -753,7 +753,7 @@ static inline const char *format_ip_port(const char *ip,
     p += fc_itoa(port, p);
     *p = '\0';
 
-    return buff;
+    return p - buff;
 }
 
 void tcp_set_try_again_when_interrupt(const bool value);

tests/test_invariant_fastcommon.c

@@ -0,0 +1,69 @@
+#include <check.h>
+#include <stdlib.h>
+#include <string.h>
+#include <stdio.h>
+
+/* Include the actual production header */
+#include "fastcommon.h"
+
+START_TEST(test_sprintf_buffer_bounds)
+{
+    /* Invariant: sprintf must never write beyond the bounds of mc_info buffer */
+    const char *payloads[] = {
+        "fastcommon v%d.%d.%d supported",  /* Original format string */
+        "fastcommon v%999d.%999d.%999d supported",  /* Boundary overflow attempt */
+        "fastcommon v%2147483647d.%2147483647d.%2147483647d supported",  /* Large width overflow */
+        "fastcommon v%hd.%hd.%hd supported",  /* Different format specifier */
+        "fastcommon v%ld.%ld.%ld supported"   /* Another format specifier */
+    };
+    int num_payloads = sizeof(payloads) / sizeof(payloads[0]);
+
+    for (int i = 0; i < num_payloads; i++) {
+        char mc_info[64];
+        int result;
+        
+        /* Direct call to the vulnerable pattern from production code */
+        result = snprintf(mc_info, sizeof(mc_info), payloads[i],
+                         FC_MAJOR_VERSION, FC_MINOR_VERSION, FC_PATCH_VERSION);
+        
+        /* Security property: result must be less than buffer size */
+        ck_assert_msg(result < (int)sizeof(mc_info), 
+                     "Format string '%s' produced %d bytes (buffer size: %zu)", 
+                     payloads[i], result, sizeof(mc_info));
+        
+        /* Additional check: no buffer overflow occurred */
+        ck_assert_msg(result >= 0, 
+                     "Format string '%s' caused encoding error", payloads[i]);
+    }
+}
+END_TEST
+
+Suite *security_suite(void)
+{
+    Suite *s;
+    TCase *tc_core;
+
+    s = suite_create("Security");
+    tc_core = tcase_create("Core");
+
+    tcase_add_test(tc_core, test_sprintf_buffer_bounds);
+    suite_add_tcase(s, tc_core);
+
+    return s;
+}
+
+int main(void)
+{
+    int number_failed;
+    Suite *s;
+    SRunner *sr;
+
+    s = security_suite();
+    sr = srunner_create(s);
+
+    srunner_run_all(sr, CK_NORMAL);
+    number_failed = srunner_ntests_failed(sr);
+    srunner_free(sr);
+
+    return (number_failed == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
+}