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
This commit is contained in:
m1ngsama 2026-01-22 14:02:05 +08:00
parent abe477f713
commit f65e8add64
2 changed files with 57 additions and 8 deletions

View file

@ -28,12 +28,33 @@ int message_load(message_t **messages, int max_messages) {
/* Use a ring buffer approach - keep only last max_messages */ /* Use a ring buffer approach - keep only last max_messages */
/* First, count total lines and seek to appropriate position */ /* 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 line_count = 0;
int start_index = 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 */ /* 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); file_pos[line_count++] = ftell(fp) - strlen(line);
} }
@ -74,6 +95,7 @@ int message_load(message_t **messages, int max_messages) {
count++; count++;
} }
free(file_pos);
fclose(fp); fclose(fp);
*messages = msg_array; *messages = msg_array;
return count; return count;

View file

@ -23,6 +23,22 @@ static int setup_host_key(ssh_bind sshbind) {
/* Check if host key exists */ /* Check if host key exists */
if (stat(HOST_KEY_FILE, &st) == 0) { if (stat(HOST_KEY_FILE, &st) == 0) {
/* 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 */ /* Load existing key */
if (ssh_bind_options_set(sshbind, SSH_BIND_OPTIONS_RSAKEY, HOST_KEY_FILE) < 0) { 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)); fprintf(stderr, "Failed to load host key: %s\n", ssh_get_error(sshbind));
@ -30,6 +46,7 @@ static int setup_host_key(ssh_bind sshbind) {
} }
return 0; return 0;
} }
}
/* Generate new key */ /* Generate new key */
printf("Generating new RSA host key...\n"); printf("Generating new RSA host key...\n");
@ -733,7 +750,17 @@ int ssh_server_start(int unused) {
/* Create thread for client */ /* Create thread for client */
pthread_t thread; 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_close(channel);
ssh_channel_free(channel); ssh_channel_free(channel);
ssh_disconnect(session); ssh_disconnect(session);
@ -742,7 +769,7 @@ int ssh_server_start(int unused) {
continue; continue;
} }
pthread_detach(thread); pthread_attr_destroy(&attr);
} }
return 0; return 0;