Skip to content

Commit

Permalink
Replace all calls to strncpy with strlcpy, use strdup more, expose st…
Browse files Browse the repository at this point in the history
…rlcat (flipperdevices#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
  • Loading branch information
CookiePLMonster authored and ofabel committed Sep 26, 2024
1 parent 25aa1ac commit 7444964
Show file tree
Hide file tree
Showing 18 changed files with 47 additions and 47 deletions.
2 changes: 1 addition & 1 deletion applications/debug/rpc_debug_app/rpc_debug_app.c
Original file line number Diff line number Diff line change
Expand Up @@ -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, "<Data empty>", buf_size);
strlcpy(buf, "<Data empty>", buf_size);
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion applications/examples/example_thermo/example_thermo.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions applications/main/ibutton/ibutton.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
}
Expand Down
4 changes: 2 additions & 2 deletions applications/main/ibutton/ibutton_i.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions applications/main/infrared/infrared_app_i.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,15 @@ 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);

} 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();
Expand Down
6 changes: 3 additions & 3 deletions applications/main/subghz/scenes/subghz_scene_save_name.c
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
#include <dolphin/dolphin.h>
#include <toolbox/name_generator.h>

#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);
Expand Down Expand Up @@ -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);

Expand All @@ -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,
Expand Down
10 changes: 3 additions & 7 deletions applications/services/desktop/animations/animation_storage.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 1 addition & 3 deletions applications/services/rpc/rpc_storage.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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));
Expand Down
31 changes: 19 additions & 12 deletions lib/mjs/common/frozen/frozen.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -263,23 +264,27 @@ 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++;
EXPECT(f->cur < f->end, JSON_STRING_INCOMPLETE);
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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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';
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down
3 changes: 1 addition & 2 deletions lib/mjs/mjs_json.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
4 changes: 2 additions & 2 deletions targets/f18/api_symbols.csv
Original file line number Diff line number Diff line change
@@ -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,,
Expand Down Expand Up @@ -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*
Expand Down
4 changes: 2 additions & 2 deletions targets/f7/api_symbols.csv
Original file line number Diff line number Diff line change
@@ -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,,
Expand Down Expand Up @@ -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*
Expand Down
2 changes: 1 addition & 1 deletion targets/f7/furi_hal/furi_hal_version.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 7444964

Please sign in to comment.