From f65e8add6493d6a1d23654ed46e911fe437b9979 Mon Sep 17 00:00:00 2001 From: m1ngsama Date: Thu, 22 Jan 2026 14:02:05 +0800 Subject: [PATCH] fix(security): enhance resource management - Convert message_load() file position array from fixed 1000 to dynamic: * Start with capacity of 1000, grow by 2x when needed * Use malloc/realloc for flexible memory management * Proper cleanup with free() after use * Graceful handling of memory allocation failures - Enhance setup_host_key() error handling: * Validate key file size (reject 0 bytes and >10MB) * Automatically regenerate if key file is empty * Verify and fix insecure permissions (must be 0600) * Better error messages with file size reporting - Improve client thread resource cleanup: * Use pthread_attr for explicit detached thread creation * Add pthread_mutex_destroy on thread creation failure * Proper cleanup order: mutex -> channel -> session -> memory * Add error logging with strerror() for thread failures These changes address: - Fixed 1000-line limit causing message truncation - Corrupted/empty key file handling - Permission race conditions - Resource leaks on thread creation failure Prevents: - DoS via large log files - Service startup failures from bad key files - Memory/handle leaks under error conditions --- src/message.c | 26 ++++++++++++++++++++++++-- src/ssh_server.c | 39 +++++++++++++++++++++++++++++++++------ 2 files changed, 57 insertions(+), 8 deletions(-) diff --git a/src/message.c b/src/message.c index 82bcb78..f89c5ba 100644 --- a/src/message.c +++ b/src/message.c @@ -28,12 +28,33 @@ int message_load(message_t **messages, int max_messages) { /* Use a ring buffer approach - keep only last max_messages */ /* First, count total lines and seek to appropriate position */ - long file_pos[1000]; /* Track positions of last 1000 lines */ + /* Use dynamic allocation to handle large log files */ + long *file_pos = NULL; + int pos_capacity = 1000; int line_count = 0; int start_index = 0; + /* Allocate initial position array */ + file_pos = malloc(pos_capacity * sizeof(long)); + if (!file_pos) { + fclose(fp); + *messages = msg_array; + return 0; + } + /* Record file positions */ - while (fgets(line, sizeof(line), fp) && line_count < 1000) { + while (fgets(line, sizeof(line), fp)) { + /* Expand array if needed */ + if (line_count >= pos_capacity) { + int new_capacity = pos_capacity * 2; + long *new_pos = realloc(file_pos, new_capacity * sizeof(long)); + if (!new_pos) { + /* Out of memory, stop scanning */ + break; + } + file_pos = new_pos; + pos_capacity = new_capacity; + } file_pos[line_count++] = ftell(fp) - strlen(line); } @@ -74,6 +95,7 @@ int message_load(message_t **messages, int max_messages) { count++; } + free(file_pos); fclose(fp); *messages = msg_array; return count; diff --git a/src/ssh_server.c b/src/ssh_server.c index 4023611..dd1f8dc 100644 --- a/src/ssh_server.c +++ b/src/ssh_server.c @@ -23,12 +23,29 @@ static int setup_host_key(ssh_bind sshbind) { /* Check if host key exists */ if (stat(HOST_KEY_FILE, &st) == 0) { - /* Load existing key */ - if (ssh_bind_options_set(sshbind, SSH_BIND_OPTIONS_RSAKEY, HOST_KEY_FILE) < 0) { - fprintf(stderr, "Failed to load host key: %s\n", ssh_get_error(sshbind)); + /* Validate file size */ + if (st.st_size == 0) { + fprintf(stderr, "Warning: Empty key file, regenerating...\n"); + unlink(HOST_KEY_FILE); + /* Fall through to generate new key */ + } else if (st.st_size > 10 * 1024 * 1024) { + /* Sanity check: key file shouldn't be > 10MB */ + fprintf(stderr, "Error: Key file too large (%lld bytes)\n", (long long)st.st_size); return -1; + } else { + /* Verify and fix permissions */ + if ((st.st_mode & 0077) != 0) { + fprintf(stderr, "Warning: Fixing insecure key file permissions\n"); + chmod(HOST_KEY_FILE, 0600); + } + + /* Load existing key */ + if (ssh_bind_options_set(sshbind, SSH_BIND_OPTIONS_RSAKEY, HOST_KEY_FILE) < 0) { + fprintf(stderr, "Failed to load host key: %s\n", ssh_get_error(sshbind)); + return -1; + } + return 0; } - return 0; } /* Generate new key */ @@ -733,7 +750,17 @@ int ssh_server_start(int unused) { /* Create thread for client */ pthread_t thread; - if (pthread_create(&thread, NULL, client_handle_session, client) != 0) { + pthread_attr_t attr; + + /* Initialize thread attributes for detached thread */ + pthread_attr_init(&attr); + pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED); + + if (pthread_create(&thread, &attr, client_handle_session, client) != 0) { + fprintf(stderr, "Thread creation failed: %s\n", strerror(errno)); + pthread_attr_destroy(&attr); + /* Clean up all resources */ + pthread_mutex_destroy(&client->ref_lock); ssh_channel_close(channel); ssh_channel_free(channel); ssh_disconnect(session); @@ -742,7 +769,7 @@ int ssh_server_start(int unused) { continue; } - pthread_detach(thread); + pthread_attr_destroy(&attr); } return 0;