From 8672a1d94cfe891c37d8f05a5a2cbc2d8a01e799 Mon Sep 17 00:00:00 2001 From: Silent Date: Sat, 7 Sep 2024 18:16:56 +0200 Subject: [PATCH] Replace all calls to strncpy with strlcpy, use strdup more, expose strlcat (#3866) strlcpy doesn't zero the buffer and ensures null termination, just like snprintf strlcat is already used by mjs and it's a safe alternative to strcat, so it should be OK to expose to apps --- .../debug/rpc_debug_app/rpc_debug_app.c | 2 +- .../rpc_debug_app_scene_input_error_code.c | 2 +- .../rpc_debug_app_scene_input_error_text.c | 2 +- ...pc_debug_app_scene_receive_data_exchange.c | 2 +- .../examples/example_thermo/example_thermo.c | 2 +- applications/main/ibutton/ibutton.c | 4 +-- applications/main/ibutton/ibutton_i.h | 4 +-- applications/main/infrared/infrared_app_i.h | 4 +-- .../scenes/infrared_scene_edit_rename.c | 4 +-- .../subghz/scenes/subghz_scene_save_name.c | 6 ++-- .../desktop/animations/animation_storage.c | 10 ++---- applications/services/rpc/rpc_storage.c | 4 +-- .../scenes/desktop_settings_scene_favorite.c | 4 +-- lib/mjs/common/frozen/frozen.c | 31 ++++++++++++------- lib/mjs/mjs_json.c | 3 +- targets/f18/api_symbols.csv | 4 +-- targets/f7/api_symbols.csv | 4 +-- targets/f7/furi_hal/furi_hal_version.c | 2 +- 18 files changed, 47 insertions(+), 47 deletions(-) diff --git a/applications/debug/rpc_debug_app/rpc_debug_app.c b/applications/debug/rpc_debug_app/rpc_debug_app.c index 1536b8918..1affeae29 100644 --- a/applications/debug/rpc_debug_app/rpc_debug_app.c +++ b/applications/debug/rpc_debug_app/rpc_debug_app.c @@ -24,7 +24,7 @@ static void rpc_debug_app_tick_event_callback(void* context) { static void rpc_debug_app_format_hex(const uint8_t* data, size_t data_size, char* buf, size_t buf_size) { if(data == NULL || data_size == 0) { - strncpy(buf, "", buf_size); + strlcpy(buf, "", buf_size); return; } diff --git a/applications/debug/rpc_debug_app/scenes/rpc_debug_app_scene_input_error_code.c b/applications/debug/rpc_debug_app/scenes/rpc_debug_app_scene_input_error_code.c index be7774881..09d58c2c9 100644 --- a/applications/debug/rpc_debug_app/scenes/rpc_debug_app_scene_input_error_code.c +++ b/applications/debug/rpc_debug_app/scenes/rpc_debug_app_scene_input_error_code.c @@ -26,7 +26,7 @@ static void rpc_debug_app_scene_input_error_code_result_callback(void* context) void rpc_debug_app_scene_input_error_code_on_enter(void* context) { RpcDebugApp* app = context; - strncpy(app->text_store, "666", TEXT_STORE_SIZE); + strlcpy(app->text_store, "666", TEXT_STORE_SIZE); text_input_set_header_text(app->text_input, "Enter error code"); text_input_set_validator( app->text_input, rpc_debug_app_scene_input_error_code_validator_callback, NULL); diff --git a/applications/debug/rpc_debug_app/scenes/rpc_debug_app_scene_input_error_text.c b/applications/debug/rpc_debug_app/scenes/rpc_debug_app_scene_input_error_text.c index b07f8f4af..cca293b66 100644 --- a/applications/debug/rpc_debug_app/scenes/rpc_debug_app_scene_input_error_text.c +++ b/applications/debug/rpc_debug_app/scenes/rpc_debug_app_scene_input_error_text.c @@ -7,7 +7,7 @@ static void rpc_debug_app_scene_input_error_text_result_callback(void* context) void rpc_debug_app_scene_input_error_text_on_enter(void* context) { RpcDebugApp* app = context; - strncpy(app->text_store, "I'm a scary error message!", TEXT_STORE_SIZE); + strlcpy(app->text_store, "I'm a scary error message!", TEXT_STORE_SIZE); text_input_set_header_text(app->text_input, "Enter error text"); text_input_set_result_callback( app->text_input, diff --git a/applications/debug/rpc_debug_app/scenes/rpc_debug_app_scene_receive_data_exchange.c b/applications/debug/rpc_debug_app/scenes/rpc_debug_app_scene_receive_data_exchange.c index 10d5e36f6..686a6f95d 100644 --- a/applications/debug/rpc_debug_app/scenes/rpc_debug_app_scene_receive_data_exchange.c +++ b/applications/debug/rpc_debug_app/scenes/rpc_debug_app_scene_receive_data_exchange.c @@ -2,7 +2,7 @@ void rpc_debug_app_scene_receive_data_exchange_on_enter(void* context) { RpcDebugApp* app = context; - strncpy(app->text_store, "Received data will appear here...", TEXT_STORE_SIZE); + strlcpy(app->text_store, "Received data will appear here...", TEXT_STORE_SIZE); text_box_set_text(app->text_box, app->text_store); text_box_set_font(app->text_box, TextBoxFontHex); diff --git a/applications/examples/example_thermo/example_thermo.c b/applications/examples/example_thermo/example_thermo.c index 895f05ce7..e5af819e9 100644 --- a/applications/examples/example_thermo/example_thermo.c +++ b/applications/examples/example_thermo/example_thermo.c @@ -257,7 +257,7 @@ static void example_thermo_draw_callback(Canvas* canvas, void* ctx) { snprintf(text_store, TEXT_STORE_SIZE, "Temperature: %+.1f%c", (double)temp, temp_units); } else { /* Or show a message that no data is available */ - strncpy(text_store, "-- No data --", TEXT_STORE_SIZE); + strlcpy(text_store, "-- No data --", TEXT_STORE_SIZE); } canvas_draw_str_aligned(canvas, middle_x, 58, AlignCenter, AlignBottom, text_store); diff --git a/applications/main/ibutton/ibutton.c b/applications/main/ibutton/ibutton.c index 765d53612..eff44c6de 100644 --- a/applications/main/ibutton/ibutton.c +++ b/applications/main/ibutton/ibutton.c @@ -183,7 +183,7 @@ bool ibutton_load_key(iButton* ibutton, bool show_error) { FuriString* tmp = furi_string_alloc(); path_extract_filename(ibutton->file_path, tmp, true); - strncpy(ibutton->key_name, furi_string_get_cstr(tmp), IBUTTON_KEY_NAME_SIZE); + strlcpy(ibutton->key_name, furi_string_get_cstr(tmp), IBUTTON_KEY_NAME_SIZE); furi_string_free(tmp); } else if(show_error) { @@ -243,7 +243,7 @@ bool ibutton_delete_key(iButton* ibutton) { } void ibutton_reset_key(iButton* ibutton) { - memset(ibutton->key_name, 0, IBUTTON_KEY_NAME_SIZE + 1); + ibutton->key_name[0] = '\0'; furi_string_reset(ibutton->file_path); ibutton_key_reset(ibutton->key); } diff --git a/applications/main/ibutton/ibutton_i.h b/applications/main/ibutton/ibutton_i.h index fc2324c63..73f108080 100644 --- a/applications/main/ibutton/ibutton_i.h +++ b/applications/main/ibutton/ibutton_i.h @@ -32,7 +32,7 @@ #define IBUTTON_APP_FILENAME_PREFIX "iBtn" #define IBUTTON_APP_FILENAME_EXTENSION ".ibtn" -#define IBUTTON_KEY_NAME_SIZE 22 +#define IBUTTON_KEY_NAME_SIZE 23 typedef enum { iButtonWriteModeInvalid, @@ -56,7 +56,7 @@ struct iButton { iButtonWriteMode write_mode; FuriString* file_path; - char key_name[IBUTTON_KEY_NAME_SIZE + 1]; + char key_name[IBUTTON_KEY_NAME_SIZE]; Submenu* submenu; ByteInput* byte_input; diff --git a/applications/main/infrared/infrared_app_i.h b/applications/main/infrared/infrared_app_i.h index 721771ef0..692cc9671 100644 --- a/applications/main/infrared/infrared_app_i.h +++ b/applications/main/infrared/infrared_app_i.h @@ -43,8 +43,8 @@ #define INFRARED_TEXT_STORE_NUM 2 #define INFRARED_TEXT_STORE_SIZE 128 -#define INFRARED_MAX_BUTTON_NAME_LENGTH 22 -#define INFRARED_MAX_REMOTE_NAME_LENGTH 22 +#define INFRARED_MAX_BUTTON_NAME_LENGTH 23 +#define INFRARED_MAX_REMOTE_NAME_LENGTH 23 #define INFRARED_APP_FOLDER EXT_PATH("infrared") #define INFRARED_APP_EXTENSION ".ir" diff --git a/applications/main/infrared/scenes/infrared_scene_edit_rename.c b/applications/main/infrared/scenes/infrared_scene_edit_rename.c index 301e936d9..ea49080a1 100644 --- a/applications/main/infrared/scenes/infrared_scene_edit_rename.c +++ b/applications/main/infrared/scenes/infrared_scene_edit_rename.c @@ -39,7 +39,7 @@ void infrared_scene_edit_rename_on_enter(void* context) { furi_check(current_button_index != InfraredButtonIndexNone); enter_name_length = INFRARED_MAX_BUTTON_NAME_LENGTH; - strncpy( + strlcpy( infrared->text_store[0], infrared_remote_get_signal_name(remote, current_button_index), enter_name_length); @@ -47,7 +47,7 @@ void infrared_scene_edit_rename_on_enter(void* context) { } else if(edit_target == InfraredEditTargetRemote) { text_input_set_header_text(text_input, "Name the remote"); enter_name_length = INFRARED_MAX_REMOTE_NAME_LENGTH; - strncpy(infrared->text_store[0], infrared_remote_get_name(remote), enter_name_length); + strlcpy(infrared->text_store[0], infrared_remote_get_name(remote), enter_name_length); FuriString* folder_path; folder_path = furi_string_alloc(); diff --git a/applications/main/subghz/scenes/subghz_scene_save_name.c b/applications/main/subghz/scenes/subghz_scene_save_name.c index 08ba0ba82..cdf218aca 100644 --- a/applications/main/subghz/scenes/subghz_scene_save_name.c +++ b/applications/main/subghz/scenes/subghz_scene_save_name.c @@ -6,7 +6,7 @@ #include #include -#define MAX_TEXT_INPUT_LEN 22 +#define MAX_TEXT_INPUT_LEN 23 void subghz_scene_save_name_text_input_callback(void* context) { furi_assert(context); @@ -39,7 +39,7 @@ void subghz_scene_save_name_on_enter(void* context) { FuriString* dir_name = furi_string_alloc(); if(!subghz_path_is_file(subghz->file_path)) { - char file_name_buf[SUBGHZ_MAX_LEN_NAME] = {0}; + char file_name_buf[SUBGHZ_MAX_LEN_NAME]; name_generator_make_auto(file_name_buf, SUBGHZ_MAX_LEN_NAME, SUBGHZ_APP_FILENAME_PREFIX); @@ -62,7 +62,7 @@ void subghz_scene_save_name_on_enter(void* context) { furi_string_set(subghz->file_path, dir_name); } - strncpy(subghz->file_name_tmp, furi_string_get_cstr(file_name), SUBGHZ_MAX_LEN_NAME); + strlcpy(subghz->file_name_tmp, furi_string_get_cstr(file_name), SUBGHZ_MAX_LEN_NAME); text_input_set_header_text(text_input, "Name signal"); text_input_set_result_callback( text_input, diff --git a/applications/services/desktop/animations/animation_storage.c b/applications/services/desktop/animations/animation_storage.c index 9ee85727b..a05525776 100644 --- a/applications/services/desktop/animations/animation_storage.c +++ b/applications/services/desktop/animations/animation_storage.c @@ -52,8 +52,7 @@ static bool animation_storage_load_single_manifest_info( if(furi_string_cmp_str(read_string, name)) break; flipper_format_set_strict_mode(file, true); - manifest_info->name = malloc(furi_string_size(read_string) + 1); - strcpy((char*)manifest_info->name, furi_string_get_cstr(read_string)); + manifest_info->name = strdup(furi_string_get_cstr(read_string)); if(!flipper_format_read_uint32(file, "Min butthurt", &u32value, 1)) break; manifest_info->min_butthurt = u32value; @@ -105,9 +104,7 @@ void animation_storage_fill_animation_list(StorageAnimationList_t* animation_lis storage_animation->manifest_info.name = NULL; if(!flipper_format_read_string(file, "Name", read_string)) break; - storage_animation->manifest_info.name = malloc(furi_string_size(read_string) + 1); - strcpy( - (char*)storage_animation->manifest_info.name, furi_string_get_cstr(read_string)); + storage_animation->manifest_info.name = strdup(furi_string_get_cstr(read_string)); if(!flipper_format_read_uint32(file, "Min butthurt", &u32value, 1)) break; storage_animation->manifest_info.min_butthurt = u32value; @@ -401,8 +398,7 @@ static bool animation_storage_load_bubbles(BubbleAnimation* animation, FlipperFo furi_string_replace_all(str, "\\n", "\n"); - FURI_CONST_ASSIGN_PTR(bubble->bubble.text, malloc(furi_string_size(str) + 1)); - strcpy((char*)bubble->bubble.text, furi_string_get_cstr(str)); + FURI_CONST_ASSIGN_PTR(bubble->bubble.text, strdup(furi_string_get_cstr(str))); if(!flipper_format_read_string(ff, "AlignH", str)) break; if(!animation_storage_cast_align(str, (Align*)&bubble->bubble.align_h)) break; diff --git a/applications/services/rpc/rpc_storage.c b/applications/services/rpc/rpc_storage.c index aedcf8c94..163535f9a 100644 --- a/applications/services/rpc/rpc_storage.c +++ b/applications/services/rpc/rpc_storage.c @@ -226,9 +226,7 @@ static void rpc_system_storage_list_root(const PB_Main* request, void* context) response.content.storage_list_response.file[i].data = NULL; response.content.storage_list_response.file[i].size = 0; response.content.storage_list_response.file[i].type = PB_Storage_File_FileType_DIR; - char* str = malloc(strlen(hard_coded_dirs[i]) + 1); - strcpy(str, hard_coded_dirs[i]); - response.content.storage_list_response.file[i].name = str; + response.content.storage_list_response.file[i].name = strdup(hard_coded_dirs[i]); } rpc_send_and_release(session, &response); diff --git a/applications/settings/desktop_settings/scenes/desktop_settings_scene_favorite.c b/applications/settings/desktop_settings/scenes/desktop_settings_scene_favorite.c index 74d09b2ac..17ef88358 100644 --- a/applications/settings/desktop_settings/scenes/desktop_settings_scene_favorite.c +++ b/applications/settings/desktop_settings/scenes/desktop_settings_scene_favorite.c @@ -209,7 +209,7 @@ bool desktop_settings_scene_favorite_on_event(void* context, SceneManagerEvent e if(dialog_file_browser_show(app->dialogs, temp_path, temp_path, &browser_options)) { submenu_reset(app->submenu); // Prevent menu from being shown when we exiting scene - strncpy( + strlcpy( curr_favorite_app->name_or_path, furi_string_get_cstr(temp_path), sizeof(curr_favorite_app->name_or_path)); @@ -219,7 +219,7 @@ bool desktop_settings_scene_favorite_on_event(void* context, SceneManagerEvent e size_t app_index = event.event - 2; const char* name = favorite_fap_get_app_name(app_index); if(name) - strncpy( + strlcpy( curr_favorite_app->name_or_path, name, sizeof(curr_favorite_app->name_or_path)); diff --git a/lib/mjs/common/frozen/frozen.c b/lib/mjs/common/frozen/frozen.c index 923e6a556..7646864c0 100644 --- a/lib/mjs/common/frozen/frozen.c +++ b/lib/mjs/common/frozen/frozen.c @@ -37,7 +37,7 @@ #ifdef _WIN32 #undef snprintf #undef vsnprintf -#define snprintf cs_win_snprintf +#define snprintf cs_win_snprintf #define vsnprintf cs_win_vsnprintf int cs_win_snprintf(char* str, size_t size, const char* format, ...); int cs_win_vsnprintf(char* str, size_t size, const char* format, va_list ap); @@ -150,7 +150,8 @@ static int json_isspace(int ch) { } static void json_skip_whitespaces(struct frozen* f) { - while(f->cur < f->end && json_isspace(*f->cur)) f->cur++; + while(f->cur < f->end && json_isspace(*f->cur)) + f->cur++; } static int json_cur(struct frozen* f) { @@ -263,15 +264,18 @@ static int json_parse_number(struct frozen* f) { f->cur += 2; EXPECT(f->cur < f->end, JSON_STRING_INCOMPLETE); EXPECT(json_isxdigit(f->cur[0]), JSON_STRING_INVALID); - while(f->cur < f->end && json_isxdigit(f->cur[0])) f->cur++; + while(f->cur < f->end && json_isxdigit(f->cur[0])) + f->cur++; } else { EXPECT(json_isdigit(f->cur[0]), JSON_STRING_INVALID); - while(f->cur < f->end && json_isdigit(f->cur[0])) f->cur++; + while(f->cur < f->end && json_isdigit(f->cur[0])) + f->cur++; if(f->cur < f->end && f->cur[0] == '.') { f->cur++; EXPECT(f->cur < f->end, JSON_STRING_INCOMPLETE); EXPECT(json_isdigit(f->cur[0]), JSON_STRING_INVALID); - while(f->cur < f->end && json_isdigit(f->cur[0])) f->cur++; + while(f->cur < f->end && json_isdigit(f->cur[0])) + f->cur++; } if(f->cur < f->end && (f->cur[0] == 'e' || f->cur[0] == 'E')) { f->cur++; @@ -279,7 +283,8 @@ static int json_parse_number(struct frozen* f) { if((f->cur[0] == '+' || f->cur[0] == '-')) f->cur++; EXPECT(f->cur < f->end, JSON_STRING_INCOMPLETE); EXPECT(json_isdigit(f->cur[0]), JSON_STRING_INVALID); - while(f->cur < f->end && json_isdigit(f->cur[0])) f->cur++; + while(f->cur < f->end && json_isdigit(f->cur[0])) + f->cur++; } } json_truncate_path(f, fstate.path_len); @@ -639,8 +644,7 @@ int json_vprintf(struct json_out* out, const char* fmt, va_list xap) { int need_len, size = sizeof(buf); char fmt2[20]; va_list ap_copy; - strncpy(fmt2, fmt, n + 1 > (int)sizeof(fmt2) ? sizeof(fmt2) : (size_t)n + 1); - fmt2[n + 1] = '\0'; + strlcpy(fmt2, fmt, sizeof(fmt2)); va_copy(ap_copy, ap); need_len = vsnprintf(pbuf, size, fmt2, ap_copy); @@ -1047,7 +1051,7 @@ int json_vscanf(const char* s, int len, const char* fmt, va_list ap) { while(fmt[i] != '\0') { if(fmt[i] == '{') { - strcat(path, "."); + strlcat(path, ".", sizeof(path)); i++; } else if(fmt[i] == '}') { if((p = strrchr(path, '.')) != NULL) *p = '\0'; @@ -1160,7 +1164,8 @@ struct json_setf_data { static int get_matched_prefix_len(const char* s1, const char* s2) { int i = 0; - while(s1[i] && s2[i] && s1[i] == s2[i]) i++; + while(s1[i] && s2[i] && s1[i] == s2[i]) + i++; return i; } @@ -1235,7 +1240,8 @@ int json_vsetf( /* Trim comma after the value that begins at object/array start */ if(s[data.prev - 1] == '{' || s[data.prev - 1] == '[') { int i = data.end; - while(i < len && json_isspace(s[i])) i++; + while(i < len && json_isspace(s[i])) + i++; if(s[i] == ',') data.end = i + 1; /* Point after comma */ } json_printf(out, "%.*s", len - data.end, s + data.end); @@ -1305,7 +1311,8 @@ struct prettify_data { }; static void indent(struct json_out* out, int level) { - while(level-- > 0) out->printer(out, " ", 2); + while(level-- > 0) + out->printer(out, " ", 2); } static void print_key(struct prettify_data* pd, const char* path, const char* name, int name_len) { diff --git a/lib/mjs/mjs_json.c b/lib/mjs/mjs_json.c index 829b3b4c0..654eeb3d6 100644 --- a/lib/mjs/mjs_json.c +++ b/lib/mjs/mjs_json.c @@ -138,8 +138,7 @@ MJS_PRIVATE mjs_err_t to_json_or_debug( vp < mjs->json_visited_stack.buf + mjs->json_visited_stack.len; vp += sizeof(mjs_val_t)) { if(*(mjs_val_t*)vp == v) { - strncpy(buf, "[Circular]", size); - len = 10; + len = strlcpy(buf, "[Circular]", size); goto clean; } } diff --git a/targets/f18/api_symbols.csv b/targets/f18/api_symbols.csv index 7f3e2f396..a8da2b366 100644 --- a/targets/f18/api_symbols.csv +++ b/targets/f18/api_symbols.csv @@ -1,5 +1,5 @@ entry,status,name,type,params -Version,+,72.4,, +Version,+,72.5,, Header,+,applications/services/bt/bt_service/bt.h,, Header,+,applications/services/bt/bt_service/bt_keys_storage.h,, Header,+,applications/services/cli/cli.h,, @@ -2598,7 +2598,7 @@ Function,+,strint_to_int64,StrintParseError,"const char*, char**, int64_t*, uint Function,+,strint_to_uint16,StrintParseError,"const char*, char**, uint16_t*, uint8_t" Function,+,strint_to_uint32,StrintParseError,"const char*, char**, uint32_t*, uint8_t" Function,+,strint_to_uint64,StrintParseError,"const char*, char**, uint64_t*, uint8_t" -Function,-,strlcat,size_t,"char*, const char*, size_t" +Function,+,strlcat,size_t,"char*, const char*, size_t" Function,+,strlcpy,size_t,"char*, const char*, size_t" Function,+,strlen,size_t,const char* Function,-,strlwr,char*,char* diff --git a/targets/f7/api_symbols.csv b/targets/f7/api_symbols.csv index c6f44f914..c294a3b7d 100644 --- a/targets/f7/api_symbols.csv +++ b/targets/f7/api_symbols.csv @@ -1,5 +1,5 @@ entry,status,name,type,params -Version,+,72.4,, +Version,+,72.5,, Header,+,applications/drivers/subghz/cc1101_ext/cc1101_ext_interconnect.h,, Header,+,applications/services/bt/bt_service/bt.h,, Header,+,applications/services/bt/bt_service/bt_keys_storage.h,, @@ -3275,7 +3275,7 @@ Function,+,strint_to_int64,StrintParseError,"const char*, char**, int64_t*, uint Function,+,strint_to_uint16,StrintParseError,"const char*, char**, uint16_t*, uint8_t" Function,+,strint_to_uint32,StrintParseError,"const char*, char**, uint32_t*, uint8_t" Function,+,strint_to_uint64,StrintParseError,"const char*, char**, uint64_t*, uint8_t" -Function,-,strlcat,size_t,"char*, const char*, size_t" +Function,+,strlcat,size_t,"char*, const char*, size_t" Function,+,strlcpy,size_t,"char*, const char*, size_t" Function,+,strlen,size_t,const char* Function,-,strlwr,char*,char* diff --git a/targets/f7/furi_hal/furi_hal_version.c b/targets/f7/furi_hal/furi_hal_version.c index bd449e599..2859ae362 100644 --- a/targets/f7/furi_hal/furi_hal_version.c +++ b/targets/f7/furi_hal/furi_hal_version.c @@ -99,7 +99,7 @@ static void furi_hal_version_set_name(const char* name) { "xFlipper %s", furi_hal_version.name); } else { - snprintf(furi_hal_version.device_name, FURI_HAL_VERSION_DEVICE_NAME_LENGTH, "xFlipper"); + strlcpy(furi_hal_version.device_name, "xFlipper", FURI_HAL_VERSION_DEVICE_NAME_LENGTH); } furi_hal_version.device_name[0] = AD_TYPE_COMPLETE_LOCAL_NAME;