Distinguish source and destination sizes for safe_strcpy when using unterminated sources, fix rtrim

Author: dpw13

examples/onvif_motion_recording_example.c

@@ -30,8 +30,8 @@ void example_enable_motion_recording(void) {
     };
 
     // Set codec and quality
-    safe_strcpy(config.codec, "h264", sizeof(config.codec));
-    safe_strcpy(config.quality, "high", sizeof(config.quality));
+    safe_strcpy(config.codec, "h264", sizeof(config.codec), 0);
+    safe_strcpy(config.quality, "high", sizeof(config.quality), 0);
 
     // Enable for a stream
     const char *stream_name = "front_door";
@@ -140,8 +140,8 @@ void example_update_configuration(void) {
         .retention_days = 60          // Increased to 60 days
     };
 
-    safe_strcpy(new_config.codec, "h265", sizeof(new_config.codec));
-    safe_strcpy(new_config.quality, "medium", sizeof(new_config.quality));
+    safe_strcpy(new_config.codec, "h265", sizeof(new_config.codec), 0);
+    safe_strcpy(new_config.quality, "medium", sizeof(new_config.quality), 0);
 
     int result = update_motion_recording_config(stream_name, &new_config);
 
@@ -199,8 +199,8 @@ void example_multiple_cameras(void) {
             .retention_days = 30
         };
 
-        safe_strcpy(config.codec, "h264", sizeof(config.codec));
-        safe_strcpy(config.quality, "high", sizeof(config.quality));
+        safe_strcpy(config.codec, "h264", sizeof(config.codec), 0);
+        safe_strcpy(config.quality, "high", sizeof(config.quality), 0);
 
         int result = enable_motion_recording(cameras[i], &config);
 

include/utils/strings.h

@@ -18,14 +18,15 @@ char *safe_strdup(const char *str);
 
 /**
  * Safe string copy with size checking. It is safe to pass an unterminated
- * string in `src`: the string will not be checked beyond `size` bytes.
+ * string in `src`: the string will not be checked beyond `src_size` bytes.
  *
  * @param dest Destination buffer
  * @param src Source string
- * @param size Size of destination buffer
- * @return 0 on success, -1 on failure
+ * @param dst_size Size of destination buffer
+ * @param src_size Size of source buffer if not null-terminated
+ * @return 0 if , -1 on failure
  */
-int safe_strcpy(char *dest, const char *src, size_t size);
+int safe_strcpy(char *dest, const char *src, size_t dst_size, size_t src_size);
 
 /**
  * Safe string concatenation with size checking
@@ -96,15 +97,19 @@ static inline const char *rtrim_pos(const char *input, size_t input_size) {
 
     const char *end;
     if (input_size > 0) {
-        end = (input + strnlen(input, input_size));
+        end = (input + strnlen(input, input_size) - 1);
     } else {
         // If input_size is zero, use the unbounded strlen
-        end = (input + strlen(input));
+        end = (input + strlen(input) - 1);
     }
+    // `end` will now point to the last non-null character. If the input is
+    // empty, `end` will be (input-1), the character *before* the terminating
+    // null.
     while (end > input && !isgraph((unsigned char)*end)) {
         end--;
     }
-    // Point to the character after the last printable character.
+    // Point to the character after the last printable character. If input was
+    // empty, this will now point to the terminating null.
     return end + 1;
 }
 

src/core/config.c

@@ -33,7 +33,7 @@ static int check_running_daemon(const char *pid_file);
 int init_daemon(const char *pid_file) {
     // Store PID file path
     if (pid_file) {
-        safe_strcpy(pid_file_path, pid_file, sizeof(pid_file_path));
+        safe_strcpy(pid_file_path, pid_file, sizeof(pid_file_path), 0);
     }
 
     // Check if daemon is already running
@@ -362,9 +362,9 @@ int stop_daemon(const char *pid_file) {
     char file_path[MAX_PATH_LENGTH];
 
     if (pid_file) {
-        safe_strcpy(file_path, pid_file, sizeof(file_path));
+        safe_strcpy(file_path, pid_file, sizeof(file_path), 0);
     } else {
-        safe_strcpy(file_path, pid_file_path, sizeof(file_path));
+        safe_strcpy(file_path, pid_file_path, sizeof(file_path), 0);
     }
 
     // Open and read PID file
@@ -511,9 +511,9 @@ int daemon_status(const char *pid_file) {
     char file_path[MAX_PATH_LENGTH];
 
     if (pid_file) {
-        safe_strcpy(file_path, pid_file, sizeof(file_path));
+        safe_strcpy(file_path, pid_file, sizeof(file_path), 0);
     } else {
-        safe_strcpy(file_path, pid_file_path, sizeof(file_path));
+        safe_strcpy(file_path, pid_file_path, sizeof(file_path), 0);
     }
 
     // First try to open the file with exclusive locking

src/core/daemon.c

@@ -56,8 +56,8 @@ static __thread char tls_log_component[64]  = {0};
 static __thread char tls_log_stream[128]    = {0};
 
 void log_set_thread_context(const char *component, const char *stream_name) {
-    safe_strcpy(tls_log_component, component ? component : "", sizeof(tls_log_component));
-    safe_strcpy(tls_log_stream, stream_name ? stream_name : "", sizeof(tls_log_stream));
+    safe_strcpy(tls_log_component, component ? component : "", sizeof(tls_log_component), 0);
+    safe_strcpy(tls_log_stream, stream_name ? stream_name : "", sizeof(tls_log_stream), 0);
 }
 
 void log_clear_thread_context(void) {
@@ -262,7 +262,7 @@ int set_log_file(const char *filename) {
     }
 
     // Store filename for potential log rotation
-    safe_strcpy(logger.log_filename, filename, sizeof(logger.log_filename));
+    safe_strcpy(logger.log_filename, filename, sizeof(logger.log_filename), 0);
 
     pthread_mutex_unlock(&logger.mutex);
 
@@ -603,7 +603,7 @@ int enable_syslog(const char *ident, int facility) {
     }
 
     // Store the identifier
-    safe_strcpy(logger.syslog_ident, ident, sizeof(logger.syslog_ident));
+    safe_strcpy(logger.syslog_ident, ident, sizeof(logger.syslog_ident), 0);
 
     // Open syslog connection
     // LOG_PID: include PID with each message

src/core/logger.c

@@ -121,7 +121,7 @@ int init_json_logger(const char *filename) {
     }
 
     // Store filename for potential log rotation
-    safe_strcpy(json_logger.log_filename, filename, sizeof(json_logger.log_filename));
+    safe_strcpy(json_logger.log_filename, filename, sizeof(json_logger.log_filename), 0);
 
     json_logger.initialized = 1;
 

src/core/logger_json.c

@@ -361,7 +361,7 @@ static int create_pid_file(const char *pid_file) {
     if (last_slash) {
         char dir_path[MAX_PATH_LENGTH];
         size_t dir_len = (size_t)(last_slash - pid_file);
-        safe_strcpy(dir_path, pid_file, dir_len);
+        safe_strcpy(dir_path, pid_file, MAX_PATH_LENGTH, dir_len);
 
         // Create directory if it doesn't exist
         struct stat st;
@@ -549,7 +549,7 @@ int main(int argc, char *argv[]) {
         } else if (strcmp(argv[i], "-c") == 0 || strcmp(argv[i], "--config") == 0) {
             if (i + 1 < argc) {
                 // Set config file path
-                safe_strcpy(custom_config_path, argv[i+1], MAX_PATH_LENGTH);
+                safe_strcpy(custom_config_path, argv[i+1], MAX_PATH_LENGTH, 0);
                 i++;
             } else {
                 log_error("Missing config file path");
@@ -756,7 +756,7 @@ int main(int argc, char *argv[]) {
 
             // Create parent directory for symlink if needed
             char parent_dir[MAX_PATH_LENGTH];
-            safe_strcpy(parent_dir, config.web_root, sizeof(parent_dir));
+            safe_strcpy(parent_dir, config.web_root, sizeof(parent_dir), 0);
             char *last_slash = strrchr(parent_dir, '/');
             if (last_slash) {
                 *last_slash = '\0';
@@ -771,7 +771,7 @@ int main(int argc, char *argv[]) {
                         config.web_root, storage_web_path, strerror(errno));
 
                 // Fall back to using the storage path directly
-                safe_strcpy(config.web_root, storage_web_path, MAX_PATH_LENGTH);
+                safe_strcpy(config.web_root, storage_web_path, MAX_PATH_LENGTH, 0);
                 log_warn("Using storage path directly for web root: %s", config.web_root);
             } else {
                 log_info("Created symlink from %s to %s", config.web_root, storage_web_path);
@@ -922,13 +922,13 @@ int main(int argc, char *argv[]) {
     };
 
     // Set CORS allowed origins, methods, and headers
-    safe_strcpy(server_config.allowed_origins, "*", sizeof(server_config.allowed_origins));
-    safe_strcpy(server_config.allowed_methods, "GET, POST, PUT, DELETE, OPTIONS", sizeof(server_config.allowed_methods));
-    safe_strcpy(server_config.allowed_headers, "Content-Type, Authorization", sizeof(server_config.allowed_headers));
+    safe_strcpy(server_config.allowed_origins, "*", sizeof(server_config.allowed_origins), 0);
+    safe_strcpy(server_config.allowed_methods, "GET, POST, PUT, DELETE, OPTIONS", sizeof(server_config.allowed_methods), 0);
+    safe_strcpy(server_config.allowed_headers, "Content-Type, Authorization", sizeof(server_config.allowed_headers), 0);
 
     if (config.web_auth_enabled) {
-        safe_strcpy(server_config.username, config.web_username, sizeof(server_config.username));
-        safe_strcpy(server_config.password, config.web_password, sizeof(server_config.password));
+        safe_strcpy(server_config.username, config.web_username, sizeof(server_config.username), 0);
+        safe_strcpy(server_config.password, config.web_password, sizeof(server_config.password), 0);
     }
 
     // Initialize HTTP server (libuv + llhttp)
@@ -1016,7 +1016,7 @@ int main(int argc, char *argv[]) {
 
             if (is_api_based) {
                 // For API-based or built-in detection (motion, onvif), use the model string as-is
-                safe_strcpy(model_path, config.streams[i].detection_model, MAX_PATH_LENGTH);
+                safe_strcpy(model_path, config.streams[i].detection_model, MAX_PATH_LENGTH, 0);
                 log_info("Using built-in/API detection for stream %s: %s",
                         config.streams[i].name, model_path);
             } else if (config.streams[i].detection_model[0] != '/') {
@@ -1046,7 +1046,7 @@ int main(int argc, char *argv[]) {
                 }
             } else {
                 // Absolute path
-                safe_strcpy(model_path, config.streams[i].detection_model, MAX_PATH_LENGTH);
+                safe_strcpy(model_path, config.streams[i].detection_model, MAX_PATH_LENGTH, 0);
 
                 // Check if file exists
                 FILE *model_file = fopen(model_path, "r");

src/core/main.c

@@ -759,7 +759,7 @@ void mqtt_set_motion_state(const char *stream_name, const detection_result_t *re
     if (!state && num_motion_states < MAX_MOTION_STREAMS) {
         state = &motion_states[num_motion_states++];
         memset(state, 0, sizeof(*state));
-        safe_strcpy(state->stream_name, stream_name, sizeof(state->stream_name));
+        safe_strcpy(state->stream_name, stream_name, sizeof(state->stream_name), 0);
     }
 
     if (!state) {
@@ -793,8 +793,7 @@ void mqtt_set_motion_state(const char *stream_name, const detection_result_t *re
             }
             if (label_idx < 0 && state->num_labels < 32) {
                 label_idx = state->num_labels++;
-                safe_strcpy(state->object_labels[label_idx], label,
-                        sizeof(state->object_labels[0]) - 1);
+                safe_strcpy(state->object_labels[label_idx], label, sizeof(state->object_labels[0]), 0);
             }
             if (label_idx >= 0) {
                 state->object_counts[label_idx]++;
@@ -921,7 +920,7 @@ static void *ha_motion_thread_func(void *arg) {
 
                 motion_states[i].motion_active = false;
                 char stream_name[256];
-                safe_strcpy(stream_name, motion_states[i].stream_name, sizeof(stream_name));
+                safe_strcpy(stream_name, motion_states[i].stream_name, sizeof(stream_name), 0);
 
                 char safe_name[256];
                 sanitize_stream_name(stream_name, safe_name, sizeof(safe_name));

src/core/mqtt_client.c

@@ -109,7 +109,7 @@ int register_component(const char *name, component_type_t type, void *context, i
 
     // Initialize the component slot
     component_info_t *component = &g_coordinator.components[slot];
-    safe_strcpy(component->name, name, sizeof(component->name));
+    safe_strcpy(component->name, name, sizeof(component->name), 0);
     component->type = type;
     atomic_store(&component->state, COMPONENT_RUNNING);
     component->context = context;

src/core/shutdown_coordinator.c

@@ -114,7 +114,7 @@ static int parse_cidr_entry(const char *cidr, int *family, unsigned char *networ
     if (strlen(cidr) >= sizeof(entry)) {
         return -1;
     }
-    safe_strcpy(entry, cidr, sizeof(entry));
+    safe_strcpy(entry, cidr, sizeof(entry), 0);
 
     char *trimmed = trim_ascii_whitespace(entry);
     if (!trimmed || trimmed[0] == '\0') {
@@ -194,7 +194,7 @@ static int normalize_allowed_login_cidrs(const char *allowed_login_cidrs,
     }
 
     char input[USER_ALLOWED_LOGIN_CIDRS_MAX];
-    safe_strcpy(input, allowed_login_cidrs, sizeof(input));
+    safe_strcpy(input, allowed_login_cidrs, sizeof(input), 0);
 
     char *saveptr = NULL;
     for (char *token = strtok_r(input, ",\n", &saveptr);
@@ -309,18 +309,18 @@ static void populate_user_from_stmt(sqlite3_stmt *stmt, user_t *user) {
     memset(user, 0, sizeof(*user));
 
     user->id = sqlite3_column_int64(stmt, 0);
-    safe_strcpy(user->username, (const char *)sqlite3_column_text(stmt, 1), sizeof(user->username));
+    safe_strcpy(user->username, (const char *)sqlite3_column_text(stmt, 1), sizeof(user->username), 0);
 
     const char *email = (const char *)sqlite3_column_text(stmt, 2);
     if (email) {
-        safe_strcpy(user->email, email, sizeof(user->email));
+        safe_strcpy(user->email, email, sizeof(user->email), 0);
     }
 
     user->role = (user_role_t)sqlite3_column_int(stmt, 3);
 
     const char *api_key = (const char *)sqlite3_column_text(stmt, 4);
     if (api_key) {
-        safe_strcpy(user->api_key, api_key, sizeof(user->api_key));
+        safe_strcpy(user->api_key, api_key, sizeof(user->api_key), 0);
     }
 
     user->created_at = sqlite3_column_int64(stmt, 5);
@@ -332,13 +332,13 @@ static void populate_user_from_stmt(sqlite3_stmt *stmt, user_t *user) {
 
     const char *allowed_tags = (const char *)sqlite3_column_text(stmt, 11);
     if (allowed_tags && allowed_tags[0] != '\0') {
-        safe_strcpy(user->allowed_tags, allowed_tags, sizeof(user->allowed_tags));
+        safe_strcpy(user->allowed_tags, allowed_tags, sizeof(user->allowed_tags), 0);
         user->has_tag_restriction = true;
     }
 
     const char *allowed_login_cidrs = (const char *)sqlite3_column_text(stmt, 12);
     if (allowed_login_cidrs && allowed_login_cidrs[0] != '\0') {
-        safe_strcpy(user->allowed_login_cidrs, allowed_login_cidrs, sizeof(user->allowed_login_cidrs));
+        safe_strcpy(user->allowed_login_cidrs, allowed_login_cidrs, sizeof(user->allowed_login_cidrs), 0);
         user->has_login_cidr_restriction = true;
     }
 }
@@ -1864,9 +1864,9 @@ int db_auth_list_user_sessions(int64_t user_id, session_t *sessions, int max_cou
         const char *token = (const char *)sqlite3_column_text(stmt, 2);
         const char *ip = (const char *)sqlite3_column_text(stmt, 7);
         const char *ua = (const char *)sqlite3_column_text(stmt, 8);
-        if (token) safe_strcpy(session->token, token, sizeof(session->token));
-        if (ip) safe_strcpy(session->ip_address, ip, sizeof(session->ip_address));
-        if (ua) safe_strcpy(session->user_agent, ua, sizeof(session->user_agent));
+        if (token) safe_strcpy(session->token, token, sizeof(session->token), 0);
+        if (ip) safe_strcpy(session->ip_address, ip, sizeof(session->ip_address), 0);
+        if (ua) safe_strcpy(session->user_agent, ua, sizeof(session->user_agent), 0);
         session->created_at = sqlite3_column_int64(stmt, 3);
         session->last_activity_at = sqlite3_column_int64(stmt, 4);
         session->idle_expires_at = sqlite3_column_int64(stmt, 5);
@@ -2079,8 +2079,8 @@ int db_auth_list_trusted_devices(int64_t user_id, trusted_device_t *devices, int
         device->user_id = sqlite3_column_int64(stmt, 1);
         const char *ip = (const char *)sqlite3_column_text(stmt, 5);
         const char *ua = (const char *)sqlite3_column_text(stmt, 6);
-        if (ip) safe_strcpy(device->ip_address, ip, sizeof(device->ip_address));
-        if (ua) safe_strcpy(device->user_agent, ua, sizeof(device->user_agent));
+        if (ip) safe_strcpy(device->ip_address, ip, sizeof(device->ip_address), 0);
+        if (ua) safe_strcpy(device->user_agent, ua, sizeof(device->user_agent), 0);
         device->created_at = sqlite3_column_int64(stmt, 2);
         device->last_used_at = sqlite3_column_int64(stmt, 3);
         device->expires_at = sqlite3_column_int64(stmt, 4);
@@ -2188,7 +2188,7 @@ int db_auth_get_totp_info(int64_t user_id, char *secret, size_t secret_size, boo
 
     const char *db_secret = (const char *)sqlite3_column_text(stmt, 0);
     if (db_secret && db_secret[0] != '\0') {
-        safe_strcpy(secret, db_secret, secret_size);
+        safe_strcpy(secret, db_secret, secret_size, 0);
     }
 
     *enabled = sqlite3_column_int(stmt, 1) != 0;
@@ -2398,7 +2398,7 @@ bool db_auth_ip_allowed_for_user(const user_t *user, const char *client_ip) {
     }
 
     char cidr_list[USER_ALLOWED_LOGIN_CIDRS_MAX];
-    safe_strcpy(cidr_list, user->allowed_login_cidrs, USER_ALLOWED_LOGIN_CIDRS_MAX);
+    safe_strcpy(cidr_list, user->allowed_login_cidrs, USER_ALLOWED_LOGIN_CIDRS_MAX, 0);
 
     char *saveptr = NULL;
     for (char *token = strtok_r(cidr_list, ",\n", &saveptr);
@@ -2434,7 +2434,7 @@ bool db_auth_stream_allowed_for_user(const user_t *user, const char *stream_tags
 
     // Tokenize stream_tags and check each against user's allowed_tags
     char stream_copy[256];
-    safe_strcpy(stream_copy, stream_tags, sizeof(stream_copy));
+    safe_strcpy(stream_copy, stream_tags, sizeof(stream_copy), 0);
 
     char *saveptr = NULL;
     char *token = strtok_r(stream_copy, ",", &saveptr);