Fix memory/null safety bugs (#74, #72, #69, #64, #60, #50, #49, #65) #80

Merged
TapTap merged 1 commits from fix/memory-safety into main 2026-07-20 18:56:45 +02:00
9 changed files with 144 additions and 18 deletions
Showing only changes of commit 4dbb2b4f8b - Show all commits
+57 -2
View File
@@ -24,9 +24,58 @@ DirectoryScanner* directory_scanner_create(char* root_directory, bool use_metada
scanner->current_path = NULL; scanner->current_path = NULL;
scanner->use_metadata = use_metadata; scanner->use_metadata = use_metadata;
scanner->chunk_size = chunk_size > 0 ? chunk_size : DESIRED_CHUNK_SIZE; scanner->chunk_size = chunk_size > 0 ? chunk_size : DESIRED_CHUNK_SIZE;
scanner->exclude_patterns = exclude_patterns; /* Deep-copy exclude patterns */
if (exclude_count > 0 && exclude_patterns != NULL) {
scanner->exclude_patterns = malloc((size_t)exclude_count * sizeof(char*));
if (scanner->exclude_patterns == NULL) {
queue_destroy(scanner->directories);
free(scanner);
return NULL;
}
for (int i = 0; i < exclude_count; i++) {
scanner->exclude_patterns[i] = str_dup(exclude_patterns[i]);
if (scanner->exclude_patterns[i] == NULL) {
for (int j = 0; j < i; j++)
free(scanner->exclude_patterns[j]);
free(scanner->exclude_patterns);
queue_destroy(scanner->directories);
free(scanner);
return NULL;
}
}
} else {
scanner->exclude_patterns = NULL;
}
scanner->exclude_count = exclude_count; scanner->exclude_count = exclude_count;
scanner->include_patterns = include_patterns;
/* Deep-copy include patterns */
if (include_count > 0 && include_patterns != NULL) {
scanner->include_patterns = malloc((size_t)include_count * sizeof(char*));
if (scanner->include_patterns == NULL) {
for (int i = 0; i < exclude_count; i++)
free(scanner->exclude_patterns[i]);
free(scanner->exclude_patterns);
queue_destroy(scanner->directories);
free(scanner);
return NULL;
}
for (int i = 0; i < include_count; i++) {
scanner->include_patterns[i] = str_dup(include_patterns[i]);
if (scanner->include_patterns[i] == NULL) {
for (int j = 0; j < i; j++)
free(scanner->include_patterns[j]);
free(scanner->include_patterns);
for (int j = 0; j < exclude_count; j++)
free(scanner->exclude_patterns[j]);
free(scanner->exclude_patterns);
queue_destroy(scanner->directories);
free(scanner);
return NULL;
}
}
} else {
scanner->include_patterns = NULL;
}
scanner->include_count = include_count; scanner->include_count = include_count;
scanner->max_size = max_size; scanner->max_size = max_size;
scanner->min_size = min_size; scanner->min_size = min_size;
@@ -42,6 +91,12 @@ void directory_scanner_destroy(DirectoryScanner* scanner) {
scanner->current_dir = NULL; scanner->current_dir = NULL;
} }
free(scanner->current_path); free(scanner->current_path);
for (int i = 0; i < scanner->exclude_count; i++)
free(scanner->exclude_patterns[i]);
free(scanner->exclude_patterns);
for (int i = 0; i < scanner->include_count; i++)
free(scanner->include_patterns[i]);
free(scanner->include_patterns);
queue_destroy(scanner->directories); queue_destroy(scanner->directories);
free(scanner); free(scanner);
} }
+12 -3
View File
@@ -1,7 +1,8 @@
#include "compression.h" #include "compression.h"
#include "data.h" #include "data.h"
#include "log.h" #include "log.h"
#include "stdlib.h" #include <stdint.h>
#include <stdlib.h>
#include "zstd.h" #include "zstd.h"
#define INITIAL_DECOMPRESS_BUF_SIZE (1024 * 1024) #define INITIAL_DECOMPRESS_BUF_SIZE (1024 * 1024)
@@ -66,8 +67,16 @@ Data* data_decompress(Data* compressed_data) {
return NULL; return NULL;
} }
size_t buf_size = size_t buf_size = INITIAL_DECOMPRESS_BUF_SIZE;
(!ZSTD_isError(dst_size) && dst_size > 0) ? (size_t)dst_size : INITIAL_DECOMPRESS_BUF_SIZE; if (!ZSTD_isError(dst_size) && dst_size > 0) {
if (dst_size > SIZE_MAX) {
log_message(LOG_LEVEL_ERROR,
"Decompressed size %llu exceeds addressable memory, using fallback buffer",
dst_size);
} else {
buf_size = (size_t)dst_size;
}
}
Data* uncompressed_data = data_create_empty(buf_size); Data* uncompressed_data = data_create_empty(buf_size);
if (!uncompressed_data) { if (!uncompressed_data) {
log_message(LOG_LEVEL_ERROR, "Failed to allocate decompression buffer"); log_message(LOG_LEVEL_ERROR, "Failed to allocate decompression buffer");
+3 -1
View File
@@ -3,7 +3,9 @@
#include "stdlib.h" #include "stdlib.h"
Data* data_create_empty(size_t data_size) { Data* data_create_empty(size_t data_size) {
void* data = malloc(data_size); /* malloc(0) is UB; allocate at least 1 byte but preserve requested size */
size_t alloc_size = data_size > 0 ? data_size : 1;
void* data = malloc(alloc_size);
if (data == NULL) { if (data == NULL) {
log_message(LOG_LEVEL_ERROR, "Could not allocate memory for empty data"); log_message(LOG_LEVEL_ERROR, "Could not allocate memory for empty data");
return NULL; return NULL;
+9
View File
@@ -138,6 +138,10 @@ static const char* status_to_string(Status status) {
} }
bool send_str(int file_descriptor, const char* data) { bool send_str(int file_descriptor, const char* data) {
if (data == NULL) {
log_message(LOG_LEVEL_ERROR, "send_str called with NULL data");
return false;
}
size_t size = strlen(data); size_t size = strlen(data);
if (!send_n_data(file_descriptor, &size, sizeof(size_t))) if (!send_n_data(file_descriptor, &size, sizeof(size_t)))
return false; return false;
@@ -151,6 +155,11 @@ char* receive_str(int file_descriptor) {
size_t size; size_t size;
if (!receive_n_data(file_descriptor, &size, sizeof(size_t))) if (!receive_n_data(file_descriptor, &size, sizeof(size_t)))
return NULL; return NULL;
if (size > MAX_STRING_SIZE) {
log_message(LOG_LEVEL_ERROR, "receive_str: size %zu exceeds maximum %zu", size,
(size_t)MAX_STRING_SIZE);
return NULL;
}
char* data = (char*)malloc(size + 1); char* data = (char*)malloc(size + 1);
if (data == NULL) if (data == NULL)
return NULL; return NULL;
+3
View File
@@ -5,6 +5,9 @@
#include <stdbool.h> #include <stdbool.h>
#include <stddef.h> #include <stddef.h>
/* Maximum allowed string size for receive_str (10 MB) */
#define MAX_STRING_SIZE (10 * 1024 * 1024)
typedef struct ssl_st SSL; typedef struct ssl_st SSL;
typedef int Status; typedef int Status;
+13 -1
View File
@@ -124,7 +124,10 @@ Client* client_connect_ssh(const char* destination, int port) {
else else
snprintf(ssh_user, sizeof(ssh_user), "%s", r.host); snprintf(ssh_user, sizeof(ssh_user), "%s", r.host);
char* ssh_argv[16]; size_t ssh_argv_max = 32;
char** ssh_argv = calloc(ssh_argv_max, sizeof(char*));
if (ssh_argv == NULL)
_exit(1);
int ac = 0; int ac = 0;
char port_str[16]; char port_str[16];
ssh_argv[ac++] = "ssh"; ssh_argv[ac++] = "ssh";
@@ -135,15 +138,24 @@ Client* client_connect_ssh(const char* destination, int port) {
ssh_argv[ac++] = "-o"; ssh_argv[ac++] = "-o";
ssh_argv[ac++] = "ControlPath=~/.cache/fastsync-%r@%h:%p"; ssh_argv[ac++] = "ControlPath=~/.cache/fastsync-%r@%h:%p";
if (port > 0 && port != 22) { if (port > 0 && port != 22) {
if ((size_t)ac + 2 >= ssh_argv_max) {
free(ssh_argv);
_exit(1);
}
ssh_argv[ac++] = "-p"; ssh_argv[ac++] = "-p";
snprintf(port_str, sizeof(port_str), "%d", port); snprintf(port_str, sizeof(port_str), "%d", port);
ssh_argv[ac++] = port_str; ssh_argv[ac++] = port_str;
} }
if ((size_t)ac + 3 >= ssh_argv_max) {
free(ssh_argv);
_exit(1);
}
ssh_argv[ac++] = ssh_user; ssh_argv[ac++] = ssh_user;
ssh_argv[ac++] = "fastsync-server"; ssh_argv[ac++] = "fastsync-server";
ssh_argv[ac++] = "--stdio"; ssh_argv[ac++] = "--stdio";
ssh_argv[ac] = NULL; ssh_argv[ac] = NULL;
execvp("ssh", ssh_argv); execvp("ssh", ssh_argv);
free(ssh_argv);
perror("exec of ssh failed"); perror("exec of ssh failed");
ssize_t wret = write(exec_pipe[1], "x", 1); ssize_t wret = write(exec_pipe[1], "x", 1);
(void)wret; (void)wret;
+20 -11
View File
@@ -10,19 +10,23 @@
#include <unistd.h> #include <unistd.h>
bool mkdir_r(const char* path) { bool mkdir_r(const char* path) {
char* path_duplicate = malloc(strlen(path) + 1); size_t path_len = strlen(path);
char* path_duplicate = malloc(path_len + 1);
if (!path_duplicate) if (!path_duplicate)
return false; return false;
strcpy(path_duplicate, path); memcpy(path_duplicate, path, path_len + 1);
char* path_current = (char*)malloc((strlen(path) + 2) * sizeof(char)); /* Buffer for building subpaths: path_len + 1 for leading '/' + 1 for null */
size_t buf_size = path_len + 2;
char* path_current = (char*)malloc(buf_size);
if (!path_current) { if (!path_current) {
free(path_duplicate); free(path_duplicate);
return false; return false;
} }
char* path_current_position = path_current; size_t pos = 0;
if (path[0] == '/') { if (path[0] == '/') {
strcpy(path_current, "/"); path_current[0] = '/';
path_current_position += 1; path_current[1] = '\0';
pos = 1;
} else { } else {
path_current[0] = '\0'; path_current[0] = '\0';
} }
@@ -31,10 +35,16 @@ bool mkdir_r(const char* path) {
const char* part = strtok_r(path_duplicate, delimiter, &saveptr); const char* part = strtok_r(path_duplicate, delimiter, &saveptr);
bool ok = true; bool ok = true;
while (part != NULL) { while (part != NULL) {
strcpy(path_current_position, part); size_t part_len = strlen(part);
path_current_position += strlen(part) * sizeof(char); if (pos + part_len + 1 >= buf_size) {
strcpy(path_current_position, "/"); ok = false;
path_current_position += sizeof(char); break;
}
memcpy(path_current + pos, part, part_len);
pos += part_len;
path_current[pos] = '/';
pos++;
path_current[pos] = '\0';
struct stat st; struct stat st;
if (stat(path_current, &st) != 0) { if (stat(path_current, &st) != 0) {
if (mkdir(path_current, 0755) != 0) { if (mkdir(path_current, 0755) != 0) {
@@ -49,7 +59,6 @@ bool mkdir_r(const char* path) {
free(path_current); free(path_current);
return ok; return ok;
} }
char* str_dup(const char* string) { char* str_dup(const char* string) {
if (string == NULL) if (string == NULL)
return NULL; return NULL;
+9
View File
@@ -23,6 +23,14 @@ static void test_data_create_empty() {
data_destroy(d); data_destroy(d);
} }
static void test_data_create_empty_zero() {
Data* d = data_create_empty(0);
EXPECT_NOT_NULL(d);
EXPECT_NOT_NULL(d->data);
EXPECT_EQ_INT((int)d->size, 0);
data_destroy(d);
}
static void test_data_create_reserve() { static void test_data_create_reserve() {
Data* d = data_create_reserve(1024); Data* d = data_create_reserve(1024);
EXPECT_NOT_NULL(d); EXPECT_NOT_NULL(d);
@@ -44,6 +52,7 @@ static void test_data_destroy_normal() {
void test_data() { void test_data() {
test_data_create(); test_data_create();
test_data_create_empty(); test_data_create_empty();
test_data_create_empty_zero();
test_data_create_reserve(); test_data_create_reserve();
test_data_destroy_null(); test_data_destroy_null();
test_data_destroy_normal(); test_data_destroy_normal();
+18
View File
@@ -169,6 +169,23 @@ static void test_receive_str_truncated() {
close(p[0]); close(p[0]);
} }
static void test_receive_str_oversized() {
int p[2];
EXPECT_EQ_INT(pipe(p), 0);
io_set_fds(p[0], p[1]);
io_set_bwlimit(0);
/* Send a size exceeding MAX_STRING_SIZE */
size_t huge = MAX_STRING_SIZE + 1;
EXPECT_TRUE(send_n_data(0, &huge, sizeof(size_t)));
char* received = receive_str(0);
EXPECT_NULL(received);
close(p[0]);
close(p[1]);
}
void test_protocol() { void test_protocol() {
test_send_receive_n_data(); test_send_receive_n_data();
test_send_receive_n_data_zero(); test_send_receive_n_data_zero();
@@ -179,4 +196,5 @@ void test_protocol() {
test_send_receive_status(); test_send_receive_status();
test_receive_n_data_truncated(); test_receive_n_data_truncated();
test_receive_str_truncated(); test_receive_str_truncated();
test_receive_str_oversized();
} }