diff --git a/c_sourcecode/src/main.c b/c_sourcecode/src/main.c index 6f82c749..360ecec2 100644 --- a/c_sourcecode/src/main.c +++ b/c_sourcecode/src/main.c @@ -250,7 +250,7 @@ int main_test(int argc, char *argv[]) for (int i = 0; i < dump_count; i++) { CRASHUPLOAD_INFO("List of dump file=%s\n", (dumps + i)->path); - process_file_entry((dumps + i)->path, argv[2], &config); + process_file_entry((dumps + i)->path, sizeof((dumps + i)->path), argv[2], &config); CRASHUPLOAD_INFO("List of dump file After process_file_entry=%s\n", (dumps + i)->path); len = strlen((dumps + i)->path); if (len > 4 && strcmp((dumps + i)->path + len - 4, ".tgz") == 0) diff --git a/c_sourcecode/src/ratelimit/ratelimit.c b/c_sourcecode/src/ratelimit/ratelimit.c index e16a9e95..6442301b 100644 --- a/c_sourcecode/src/ratelimit/ratelimit.c +++ b/c_sourcecode/src/ratelimit/ratelimit.c @@ -121,7 +121,7 @@ int is_upload_limit_reached(const char *file) } /* Validate numeric content of FIRST line */ - for (size_t i = 0; first_line_data[i] != '\0' && first_line_data[i] != '\n' && i < sizeof(first_line_data); i++) + for (size_t i = 0; i < sizeof(first_line_data) && first_line_data[i] != '\0' && first_line_data[i] != '\n'; i++) { if (!isdigit((unsigned char)first_line_data[i])) { @@ -197,7 +197,7 @@ int is_recovery_time_reached(const char *deny_file) fclose(fp); /* Validate numeric content with bounds check */ - for (size_t i = 0; buf[i] != '\0' && buf[i] != '\n' && i < sizeof(buf); i++) + for (size_t i = 0; i < sizeof(buf) && buf[i] != '\0' && buf[i] != '\n'; i++) { if (!isdigit((unsigned char)buf[i])) return ALLOW_UPLOAD; diff --git a/c_sourcecode/src/scanner/scanner.c b/c_sourcecode/src/scanner/scanner.c index f6a002b5..22aed2d4 100644 --- a/c_sourcecode/src/scanner/scanner.c +++ b/c_sourcecode/src/scanner/scanner.c @@ -605,8 +605,8 @@ int processCrashTelemetryInfo(const char *rawfile, const char *log_path, bool t2 snprintf(Appname, sizeof(Appname), "%s", us + 1); size_t pnamelen = (size_t)(us - containerName); - if (pnamelen >= sizeof(ProcessName)) - pnamelen = sizeof(ProcessName) - 1; + if (pnamelen >= sizeof(containerName)) + pnamelen = sizeof(containerName) - 1; memcpy(ProcessName, containerName, pnamelen); ProcessName[pnamelen] = '\0'; } @@ -661,11 +661,11 @@ int processCrashTelemetryInfo(const char *rawfile, const char *log_path, bool t2 * * Returns 0 on success, non-zero on error. */ -int process_file_entry(char *fullpath, char *dump_type, const config_t *config) +int process_file_entry(char *fullpath, size_t fullpath_cap, char *dump_type, const config_t *config) { int sanitize_ret = -1; char sanitized[256] = {0}; - if (!fullpath || !dump_type || !config) + if (!fullpath || fullpath_cap == 0 || !dump_type || !config) { CRASHUPLOAD_INFO("process_file_entry():Invalid Parameter\n"); return -1; @@ -683,7 +683,8 @@ int process_file_entry(char *fullpath, char *dump_type, const config_t *config) char basename[PATH_MAX_LEN]; char fullcopy[PATH_MAX_LEN]; - if (strlen(fullpath) >= PATH_MAX_LEN) + size_t fullpath_len = strnlen(fullpath, fullpath_cap); + if (fullpath_len >= fullpath_cap || fullpath_len >= PATH_MAX_LEN) { CRASHUPLOAD_INFO("process_file_entry(): File name is cross max length:%d\n", PATH_MAX_LEN); return -1; @@ -734,6 +735,11 @@ int process_file_entry(char *fullpath, char *dump_type, const config_t *config) CRASHUPLOAD_INFO("Path too long after sanitization; skipping rename\n"); return -1; } + if (strlen(newfull) >= fullpath_cap) + { + CRASHUPLOAD_INFO("Renamed path does not fit in caller buffer; skipping rename\n"); + return -1; + } /* Use rename() for atomic move; if target exists, choose to overwrite */ if (rename(fullpath, newfull) != 0) { @@ -749,7 +755,7 @@ int process_file_entry(char *fullpath, char *dump_type, const config_t *config) else { CRASHUPLOAD_INFO("Renamed After retry %s -> %s\n", fullpath, newfull); - strcpy(fullpath, newfull); + snprintf(fullpath, fullpath_cap, "%s", newfull); CRASHUPLOAD_INFO("After rename fullpath buffer updated=%s=\n", fullpath); } } @@ -762,7 +768,7 @@ int process_file_entry(char *fullpath, char *dump_type, const config_t *config) else { CRASHUPLOAD_INFO("Renamed %s -> %s\n", fullpath, newfull); - strcpy(fullpath, newfull); + snprintf(fullpath, fullpath_cap, "%s", newfull); CRASHUPLOAD_INFO("After rename fullpath buffer updated=%s=\n", fullpath); } /* set fullpath to newfull for subsequent processing */ diff --git a/c_sourcecode/src/scanner/scanner.h b/c_sourcecode/src/scanner/scanner.h index 854ecc42..8ff00872 100644 --- a/c_sourcecode/src/scanner/scanner.h +++ b/c_sourcecode/src/scanner/scanner.h @@ -37,11 +37,12 @@ int scanner_find_dumps(const char *path, dump_file_t **dumps, int *count, const /** * @brief Sanitise and rename a single dump file entry in-place. * @param fullpath Path to the dump file; may be updated on rename. + * @param fullpath_cap Capacity of @p fullpath in bytes, including the terminator. * @param dump_type String dump-type label ("coredump" or "minidump"). * @param config Application config. * @return 0 on success. */ -int process_file_entry(char *fullpath, char *dump_type, const config_t *config); +int process_file_entry(char *fullpath, size_t fullpath_cap, char *dump_type, const config_t *config); /** * @brief Extract the process name component from a dump file path. diff --git a/unittest/mainapp_gmock.cpp b/unittest/mainapp_gmock.cpp index b6d924e5..1dff9ee5 100644 --- a/unittest/mainapp_gmock.cpp +++ b/unittest/mainapp_gmock.cpp @@ -731,8 +731,8 @@ int scanner_find_dumps(const char *path, dump_file_t **dumps, int *count) { /** * Mock: Process file entry */ -int process_file_entry(char *fullpath, char *dump_type, const config_t *config) { - if (!fullpath || !dump_type || !config) { +int process_file_entry(char *fullpath, size_t fullpath_cap, char *dump_type, const config_t *config) { + if (!fullpath || fullpath_cap == 0 || !dump_type || !config) { return -1; } diff --git a/unittest/scanner_gmock.cpp b/unittest/scanner_gmock.cpp index 9e217f26..f5cad9ef 100644 --- a/unittest/scanner_gmock.cpp +++ b/unittest/scanner_gmock.cpp @@ -581,7 +581,7 @@ void scanner_cleanup(void) * char dump_type[] = "0"; * config_t config; * - * int result = process_file_entry(fullpath, dump_type, &config); + * int result = process_file_entry(fullpath, sizeof(fullpath), dump_type, &config); * * // Should return early since it's not a regular file * EXPECT_EQ(result, 0); diff --git a/unittest/scanner_gtest.cpp b/unittest/scanner_gtest.cpp index 11a6b904..09a4e7af 100644 --- a/unittest/scanner_gtest.cpp +++ b/unittest/scanner_gtest.cpp @@ -61,7 +61,7 @@ int scanner_find_dumps(const char *path, dump_file_t **dumps, int *count, const int scanner_get_sorted_dumps(dump_file_t **dumps, int *count); void scanner_cleanup(void); void scanner_update_test_state(dump_file_t *dumps, int count); -int process_file_entry(char *fullpath, char *dump_type, const config_t *config); +int process_file_entry(char *fullpath, size_t fullpath_cap, char *dump_type, const config_t *config); int sanitize_filename_preserve_container(const char *fname, char *out, size_t outsz); char *extract_pname(const char *filepath); char *extract_appname(const char *filepath); @@ -868,14 +868,14 @@ TEST_F(ScannerTest, ProcessTelemetry_LogMapperWithNoEqualsLine_Handled) { TEST_F(ScannerTest, ProcessFileEntry_NullFullpath) { char dump_type[] = "0"; - int result = process_file_entry(nullptr, dump_type, &test_config); + int result = process_file_entry(nullptr, 0, dump_type, &test_config); EXPECT_EQ(result, -1); } TEST_F(ScannerTest, ProcessFileEntry_NullDumpType) { char fullpath[] = "/tmp/test.dmp"; - int result = process_file_entry(fullpath, nullptr, &test_config); + int result = process_file_entry(fullpath, sizeof(fullpath), nullptr, &test_config); EXPECT_EQ(result, -1); } @@ -883,12 +883,12 @@ TEST_F(ScannerTest, ProcessFileEntry_NullConfig) { char fullpath[] = "/tmp/test.dmp"; char dump_type[] = "0"; - int result = process_file_entry(fullpath, dump_type, nullptr); + int result = process_file_entry(fullpath, sizeof(fullpath), dump_type, nullptr); EXPECT_EQ(result, -1); } TEST_F(ScannerTest, ProcessFileEntry_AllNullParameters) { - int result = process_file_entry(nullptr, nullptr, nullptr); + int result = process_file_entry(nullptr, 0, nullptr, nullptr); EXPECT_EQ(result, -1); } @@ -900,7 +900,7 @@ TEST_F(ScannerTest, ProcessFileEntry_NotRegularFile) { // Directory, not a file set_mock_is_regular_file_behavior(0); - int result = process_file_entry(fullpath, dump_type, &test_config); + int result = process_file_entry(fullpath, sizeof(fullpath), dump_type, &test_config); EXPECT_EQ(result, 0); // Returns 0 but doesn't process } @@ -913,7 +913,7 @@ TEST_F(ScannerTest, ProcessFileEntry_ValidFile) { set_mock_is_regular_file_behavior(1); - int result = process_file_entry(fullpath, dump_type, &test_config); + int result = process_file_entry(fullpath, sizeof(fullpath), dump_type, &test_config); // Success or error depending on file operations EXPECT_GE(result, -1); } @@ -928,7 +928,7 @@ TEST_F(ScannerTest, ProcessFileEntry_FileWithInvalidChars) { set_mock_is_regular_file_behavior(1); - int result = process_file_entry(fullpath, dump_type, &test_config); + int result = process_file_entry(fullpath, sizeof(fullpath), dump_type, &test_config); // Should attempt to rename or delete EXPECT_GE(result, -1); } @@ -942,7 +942,7 @@ TEST_F(ScannerTest, ProcessFileEntry_DumpTypeNonZero) { set_mock_is_regular_file_behavior(1); - int result = process_file_entry(fullpath, dump_type, &test_config); + int result = process_file_entry(fullpath, sizeof(fullpath), dump_type, &test_config); // Should process but not call telemetry EXPECT_GE(result, -1); } @@ -956,7 +956,7 @@ TEST_F(ScannerTest, ProcessFileEntry_ContainerDelimiterFile) { set_mock_is_regular_file_behavior(1); - int result = process_file_entry(fullpath, dump_type, &test_config); + int result = process_file_entry(fullpath, sizeof(fullpath), dump_type, &test_config); EXPECT_GE(result, -1); } @@ -970,7 +970,7 @@ TEST_F(ScannerTest, ProcessFileEntry_PathTooLong) { snprintf(fullpath1, sizeof(fullpath1), "%s/%s", test_dump_dir,fullpath); set_mock_is_regular_file_behavior(1); - int result = process_file_entry(fullpath1, dump_type, &test_config); + int result = process_file_entry(fullpath1, sizeof(fullpath1), dump_type, &test_config); EXPECT_EQ(result, -1); } @@ -984,7 +984,7 @@ TEST_F(ScannerTest, ProcessFileEntry_EmptySanitizedName) { set_mock_is_regular_file_behavior(1); - int result = process_file_entry(fullpath, dump_type, &test_config); + int result = process_file_entry(fullpath, sizeof(fullpath), dump_type, &test_config); // Should attempt to delete the file EXPECT_GE(result, -1); } @@ -998,7 +998,7 @@ TEST_F(ScannerTest, ProcessFileEntry_RenameScenario) { set_mock_is_regular_file_behavior(1); - int result = process_file_entry(fullpath, dump_type, &test_config); + int result = process_file_entry(fullpath, sizeof(fullpath), dump_type, &test_config); // Should attempt to rename EXPECT_GE(result, -1); } @@ -1658,7 +1658,7 @@ TEST_F(ScannerTest, ProcessFileEntry_NoSlashInPath_DirnameDot) { char fullpath[] = "nodirname_test.dmp"; char dump_type[] = "1"; set_mock_is_regular_file_behavior(1); - int result = process_file_entry(fullpath, dump_type, &test_config); + int result = process_file_entry(fullpath, sizeof(fullpath), dump_type, &test_config); EXPECT_GE(result, -1); } @@ -1674,7 +1674,7 @@ TEST_F(ScannerTest, ProcessFileEntry_RenameNeeded_DumpTypeNonZero) { char dump_type[] = "1"; snprintf(fullpath, sizeof(fullpath), "%s/bad*rename.dmp", test_dump_dir); set_mock_is_regular_file_behavior(1); - int result = process_file_entry(fullpath, dump_type, &test_config); + int result = process_file_entry(fullpath, sizeof(fullpath), dump_type, &test_config); EXPECT_GE(result, -1); }