mkdir_r() has potential buffer overflow in path construction loop #50

Closed
opened 2026-07-20 17:13:07 +02:00 by TapTap · 0 comments
Owner

Description

In src/shared/utils.c lines 11-49, the mkdir_r() function allocates a buffer of strlen(path) + 2 bytes for building directory paths, but then uses strcpy() to append path components without bounds checking:

char* path_current = (char*)malloc((strlen(path) + 2) * sizeof(char));
// ...
while (part != NULL) {
    strcpy(path_current_position, part);        // potential overflow
    path_current_position += strlen(part) * sizeof(char);
    strcpy(path_current_position, "/");          // potential overflow
    path_current_position += sizeof(char);
    // ...
    part = strtok(NULL, delimiter);
}

Each strcpy writes without checking if the remaining buffer is large enough. The allocation strlen(path) + 2 barely fits the original path plus a null terminator, but each intermediate directory component is built incrementally by appending to the buffer, potentially overflowing if the path has directory separators in unexpected positions.

Impact

While the specific use case in this codebase is likely safe (paths come from the system and strtok segments the path), this is a latent memory safety bug. An attacker-controlled or unusually structured path could trigger a buffer overflow.

Location

src/shared/utils.c:16-48

Suggested Fix

Replace the manual buffer arithmetic with snprintf() or use asprintf() for safe path construction. Alternatively, allocate using strlen(path) + strlen(path) + 2 (double the path length) for safety margin.

Better: Use a simpler recursive approach that creates directories one at a time, or use snprintf:

char result[PATH_MAX];
int written = snprintf(result, sizeof(result), "%.*s/%.*s", prefix_len, path, part_len, part);

Severity

Medium

Category

Bug


This issue was automatically generated by the issue-creator agent.

## Description In `src/shared/utils.c` lines 11-49, the `mkdir_r()` function allocates a buffer of `strlen(path) + 2` bytes for building directory paths, but then uses `strcpy()` to append path components without bounds checking: ```c char* path_current = (char*)malloc((strlen(path) + 2) * sizeof(char)); // ... while (part != NULL) { strcpy(path_current_position, part); // potential overflow path_current_position += strlen(part) * sizeof(char); strcpy(path_current_position, "/"); // potential overflow path_current_position += sizeof(char); // ... part = strtok(NULL, delimiter); } ``` Each `strcpy` writes without checking if the remaining buffer is large enough. The allocation `strlen(path) + 2` barely fits the original path plus a null terminator, but each intermediate directory component is built incrementally by appending to the buffer, potentially overflowing if the path has directory separators in unexpected positions. ## Impact While the specific use case in this codebase is likely safe (paths come from the system and `strtok` segments the path), this is a latent memory safety bug. An attacker-controlled or unusually structured path could trigger a buffer overflow. ## Location `src/shared/utils.c:16-48` ## Suggested Fix Replace the manual buffer arithmetic with `snprintf()` or use `asprintf()` for safe path construction. Alternatively, allocate using `strlen(path) + strlen(path) + 2` (double the path length) for safety margin. Better: Use a simpler recursive approach that creates directories one at a time, or use `snprintf`: ```c char result[PATH_MAX]; int written = snprintf(result, sizeof(result), "%.*s/%.*s", prefix_len, path, part_len, part); ``` ## Severity Medium ## Category Bug --- _This issue was automatically generated by the issue-creator agent._
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: TapTap/FastSync#50