Fixing SONAR issues (#13)

* Secure cleanup of FuriString holding token secret in CLI
* Few refactoring
* Working on SONAR issues

.github/workflows/sonarcloud.yml

@@ -6,6 +6,7 @@ on:
   push:
     branches:
       - master
+      - vnext
 jobs:
   build:
     name: Build and analyze

FAQ.md

@@ -22,7 +22,7 @@ Pull the repo with recursive submodule initialization and then run `./build.ps1`
 
 At first start app will create new config file (default location is [`/ext/apps/Misc/totp.conf`](https://github.com/akopachov/flipper-zero_authenticator/blob/master/totp/services/config/config.c#:~:text=%23define%20CONFIG_FILE_DIRECTORY_PATH,totp.conf%22)).
 
-Detailed description of file format can be found [here](.github/conf-file_description.md)
+Detailed description of file format can be found [here](docs/conf-file_description.md)
 
 ## Is there a CLI?
 
@@ -47,7 +47,7 @@ Flipper Zero clock has known clock drift problem. So there is a chance that cloc
 
 ### Timezone is not correct
 
-Because of Flipper Zero API doesn't provide an access to timezone offset it is necessary to set it manually for correct TOTP tokens generation. You may find you timezone offset (or another name is "UTC offset") [here](https://www.timeanddate.com/time/zone/timezone/utc) or on any other website found in google. Then set it either in [conf file](.github/conf-file_description.md) or via setting menu of Flipper Authenticator.
+Because of Flipper Zero API doesn't provide an access to timezone offset it is necessary to set it manually for correct TOTP tokens generation. You may find you timezone offset (or another name is "UTC offset") [here](https://www.timeanddate.com/time/zone/timezone/utc) or on any other website found in google. Then set it either in [conf file](docs/conf-file_description.md) or via setting menu of Flipper Authenticator.
 
 ### Token secret is not correct
 

README.md

@@ -7,7 +7,7 @@
 ![contributions - welcome](https://img.shields.io/badge/contributions-welcome-blue)
 [![Discord server](https://img.shields.io/discord/937479784148115456)](https://discord.com/channels/937479784148115456)
 
-![Screenshot](.github/assets/screenshot_1.png)
+![Screenshot](docs/assets/screenshot_1.png)
 
 ## Description
 

.github/conf-file_description.md → docs/conf-file_description.md

@@ -102,4 +102,4 @@ TokenName: Verifyr sha512 8
 TokenSecret: 3KKGABEJ4CKS5AHBZBDHFFNKUZHN6D7TKUGI3T7SHEUBAMIAPBUBWQNCMEEGEJX2LF23PYAFUCSRNVQ2ENOQWLHISCOJQCU2SCND4CI
 TokenAlgo: sha512
 TokenDigits: 8
-```
+```

totp/scenes/add_new_token/totp_input_text.c

@@ -2,6 +2,16 @@
 #include <gui/view_i.h>
 #include "../../types/common.h"
 
+size_t strnlen(const char* s, size_t maxlen) {
+    size_t len;
+
+    for(len = 0; len < maxlen; len++, s++) {
+        if(!*s) break;
+    }
+
+    return len;
+}
+
 void view_draw(View* view, Canvas* canvas) {
     furi_assert(view);
     if(view->draw_callback) {
@@ -32,10 +42,14 @@ static void commit_text_input_callback(void* context) {
     InputTextSceneState* text_input_state = (InputTextSceneState*)context;
     if(text_input_state->callback != 0) {
         InputTextSceneCallbackResult* result = malloc(sizeof(InputTextSceneCallbackResult));
-        result->user_input_length = strlen(text_input_state->text_input_buffer);
+        result->user_input_length =
+            strnlen(text_input_state->text_input_buffer, INPUT_BUFFER_SIZE);
         result->user_input = malloc(result->user_input_length + 1);
         result->callback_data = text_input_state->callback_data;
-        strcpy(result->user_input, text_input_state->text_input_buffer);
+        strlcpy(
+            result->user_input,
+            text_input_state->text_input_buffer,
+            result->user_input_length + 1);
         text_input_state->callback(result);
     }
 }

totp/scenes/add_new_token/totp_input_text.h

@@ -10,7 +10,7 @@
 
 typedef struct {
     char* user_input;
-    uint8_t user_input_length;
+    size_t user_input_length;
     void* callback_data;
 } InputTextSceneCallbackResult;
 

totp/scenes/add_new_token/totp_scene_add_new_token.c

@@ -25,9 +25,9 @@ typedef enum {
 
 typedef struct {
     char* token_name;
-    uint8_t token_name_length;
+    size_t token_name_length;
     char* token_secret;
-    uint8_t token_secret_length;
+    size_t token_secret_length;
     bool saved;
     Control selected_control;
     InputTextSceneContext* token_name_input_context;
@@ -235,7 +235,10 @@ bool totp_scene_add_new_token_handle_event(PluginEvent* const event, PluginState
 
                     if(token_secret_set) {
                         tokenInfo->name = malloc(scene_state->token_name_length + 1);
-                        strcpy(tokenInfo->name, scene_state->token_name);
+                        strlcpy(
+                            tokenInfo->name,
+                            scene_state->token_name,
+                            scene_state->token_name_length + 1);
                         tokenInfo->algo = scene_state->algo;
                         tokenInfo->digits = scene_state->digits_count;
 

totp/scenes/generate_token/totp_scene_generate_token.c

@@ -9,6 +9,7 @@
 #include "../../services/totp/totp.h"
 #include "../../services/config/config.h"
 #include "../../services/crypto/crypto.h"
+#include "../../services/crypto/memset_s.h"
 #include "../scene_director.h"
 #include "../token_menu/totp_scene_token_menu.h"
 
@@ -95,7 +96,7 @@ void update_totp_params(PluginState* const plugin_state) {
     }
 }
 
-void totp_scene_generate_token_init(PluginState* plugin_state) {
+void totp_scene_generate_token_init(const PluginState* plugin_state) {
     UNUSED(plugin_state);
 }
 
@@ -180,7 +181,7 @@ void totp_scene_generate_token_render(Canvas* const canvas, PluginState* plugin_
                              ->data);
 
         if(tokenInfo->token != NULL && tokenInfo->token_length > 0) {
-            uint8_t key_length;
+            size_t key_length;
             uint8_t* key = totp_crypto_decrypt(
                 tokenInfo->token, tokenInfo->token_length, &plugin_state->iv[0], &key_length);
 
@@ -195,7 +196,7 @@ void totp_scene_generate_token_render(Canvas* const canvas, PluginState* plugin_
                     TOKEN_LIFETIME),
                 scene_state->last_code,
                 tokenInfo->digits);
-            memset(key, 0, key_length);
+            memset_s(key, sizeof(key), 0, key_length);
             free(key);
         } else {
             i_token_to_str(0, scene_state->last_code, tokenInfo->digits);
@@ -265,7 +266,9 @@ void totp_scene_generate_token_render(Canvas* const canvas, PluginState* plugin_
     }
 }
 
-bool totp_scene_generate_token_handle_event(PluginEvent* const event, PluginState* plugin_state) {
+bool totp_scene_generate_token_handle_event(
+    const PluginEvent* const event,
+    PluginState* plugin_state) {
     if(event->type == EventTypeKey) {
         if(event->input.type == InputTypeLong && event->input.key == InputKeyBack) {
             return false;
@@ -314,11 +317,10 @@ void totp_scene_generate_token_deactivate(PluginState* plugin_state) {
     if(plugin_state->current_scene_state == NULL) return;
     SceneState* scene_state = (SceneState*)plugin_state->current_scene_state;
 
-    free(scene_state->last_code);
     free(scene_state);
     plugin_state->current_scene_state = NULL;
 }
 
-void totp_scene_generate_token_free(PluginState* plugin_state) {
+void totp_scene_generate_token_free(const PluginState* plugin_state) {
     UNUSED(plugin_state);
 }

totp/scenes/generate_token/totp_scene_generate_token.h

@@ -10,11 +10,13 @@ typedef struct {
     uint8_t current_token_index;
 } GenerateTokenSceneContext;
 
-void totp_scene_generate_token_init(PluginState* plugin_state);
+void totp_scene_generate_token_init(const PluginState* plugin_state);
 void totp_scene_generate_token_activate(
     PluginState* plugin_state,
     const GenerateTokenSceneContext* context);
 void totp_scene_generate_token_render(Canvas* const canvas, PluginState* plugin_state);
-bool totp_scene_generate_token_handle_event(PluginEvent* const event, PluginState* plugin_state);
+bool totp_scene_generate_token_handle_event(
+    const PluginEvent* const event,
+    PluginState* plugin_state);
 void totp_scene_generate_token_deactivate(PluginState* plugin_state);
-void totp_scene_generate_token_free(PluginState* plugin_state);
+void totp_scene_generate_token_free(const PluginState* plugin_state);

totp/services/cli/cli_helpers.c

@@ -11,7 +11,6 @@ bool totp_cli_ensure_authenticated(PluginState* plugin_state, Cli* cli) {
         }
 
         TOTP_CLI_DELETE_LAST_LINE();
-        fflush(stdout);
 
         if(plugin_state->current_scene == TotpSceneAuthentication) {
             return false;

totp/services/cli/cli_helpers.h

@@ -13,14 +13,26 @@
 #define DOCOPT_OPTIONS "[options]"
 #define DOCOPT_DEFAULT(val) "[default: " val "]"
 
-#define TOTP_CLI_PRINTF(format, ...)                                 \
-    _Pragma(STRINGIFY(GCC diagnostic push));                         \
-    _Pragma(STRINGIFY(GCC diagnostic ignored "-Wdouble-promotion")); \
-    printf(format, ##__VA_ARGS__);                                   \
-    _Pragma(STRINGIFY(GCC diagnostic pop));
-
-#define TOTP_CLI_DELETE_LAST_LINE() TOTP_CLI_PRINTF("\033[A\33[2K\r")
-#define TOTP_CLI_DELETE_CURRENT_LINE() TOTP_CLI_PRINTF("\33[2K\r")
+#define TOTP_CLI_PRINTF(format, ...)                                        \
+    do {                                                                    \
+        _Pragma(STRINGIFY(GCC diagnostic push))                             \
+            _Pragma(STRINGIFY(GCC diagnostic ignored "-Wdouble-promotion")) \
+                printf(format, ##__VA_ARGS__);                              \
+        _Pragma(STRINGIFY(GCC diagnostic pop))                              \
+    } while(false)
+
+#define TOTP_CLI_DELETE_LAST_LINE()    \
+    TOTP_CLI_PRINTF("\033[A\33[2K\r"); \
+    fflush(stdout)
+
+#define TOTP_CLI_DELETE_CURRENT_LINE() \
+    TOTP_CLI_PRINTF("\33[2K\r");       \
+    fflush(stdout)
+
+#define TOTP_CLI_DELETE_LAST_CHAR() \
+    TOTP_CLI_PRINTF("\b \b");       \
+    fflush(stdout)
+
 #define TOTP_CLI_PRINT_INVALID_ARGUMENTS() \
     TOTP_CLI_PRINTF(                       \
         "Invalid command arguments. use \"help\" command to get list of available commands")

totp/services/cli/commands/add/add.c

@@ -79,10 +79,16 @@ void totp_cli_command_add_docopt_options() {
         TOTP_CLI_COMMAND_ADD_ARG_UNSECURE_PREFIX) "             Show console user input as-is without masking\r\n");
 }
 
+static void furi_string_secure_free(FuriString* str) {
+    for(long i = furi_string_size(str) - 1; i >= 0; i--) {
+        furi_string_set_char(str, i, '\0');
+    }
+
+    furi_string_free(str);
+}
+
 void totp_cli_command_add_handle(PluginState* plugin_state, FuriString* args, Cli* cli) {
     FuriString* temp_str = furi_string_alloc();
-    const char* temp_cstr;
-
     TokenInfo* token_info = token_info_alloc();
 
     // Reading token name
@@ -93,9 +99,9 @@ void totp_cli_command_add_handle(PluginState* plugin_state, FuriString* args, Cl
         return;
     }
 
-    temp_cstr = furi_string_get_cstr(temp_str);
-    token_info->name = malloc(strlen(temp_cstr) + 1);
-    strcpy(token_info->name, temp_cstr);
+    size_t temp_cstr_len = furi_string_size(temp_str);
+    token_info->name = malloc(temp_cstr_len + 1);
+    strlcpy(token_info->name, furi_string_get_cstr(temp_str), temp_cstr_len + 1);
 
     // Read optional arguments
     bool mask_user_input = true;
@@ -146,13 +152,15 @@ void totp_cli_command_add_handle(PluginState* plugin_state, FuriString* args, Cl
     uint8_t c;
     while(cli_read(cli, &c, 1) == 1) {
         if(c == CliSymbolAsciiEsc) {
+            // Some keys generating escape-sequences
+            // We need to ignore them as we case about alpha-numerics only
             uint8_t c2;
             cli_read_timeout(cli, &c2, 1, 0);
             cli_read_timeout(cli, &c2, 1, 0);
         } else if(c == CliSymbolAsciiETX) {
             TOTP_CLI_DELETE_CURRENT_LINE();
-            TOTP_CLI_PRINTF("Cancelled by user");
-            furi_string_free(temp_str);
+            TOTP_CLI_PRINTF("Cancelled by user\r\n");
+            furi_string_secure_free(temp_str);
             token_info_free(token_info);
             return;
         } else if((c >= '0' && c <= '9') || (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z')) {
@@ -166,8 +174,7 @@ void totp_cli_command_add_handle(PluginState* plugin_state, FuriString* args, Cl
         } else if(c == CliSymbolAsciiBackspace || c == CliSymbolAsciiDel) {
             size_t temp_str_size = furi_string_size(temp_str);
             if(temp_str_size > 0) {
-                TOTP_CLI_PRINTF("\b \b");
-                fflush(stdout);
+                TOTP_CLI_DELETE_LAST_CHAR();
                 furi_string_left(temp_str, temp_str_size - 1);
             }
         } else if(c == CliSymbolAsciiCR) {
@@ -176,25 +183,26 @@ void totp_cli_command_add_handle(PluginState* plugin_state, FuriString* args, Cl
         }
     }
 
-    temp_cstr = furi_string_get_cstr(temp_str);
-
     TOTP_CLI_DELETE_LAST_LINE();
 
     if(!totp_cli_ensure_authenticated(plugin_state, cli)) {
-        furi_string_free(temp_str);
+        furi_string_secure_free(temp_str);
         token_info_free(token_info);
         return;
     }
 
-    if(!token_info_set_secret(token_info, temp_cstr, strlen(temp_cstr), plugin_state->iv)) {
+    if(!token_info_set_secret(
+           token_info,
+           furi_string_get_cstr(temp_str),
+           furi_string_size(temp_str),
+           plugin_state->iv)) {
         TOTP_CLI_PRINTF("Token secret seems to be invalid and can not be parsed\r\n");
-        furi_string_free(temp_str);
+        furi_string_secure_free(temp_str);
         token_info_free(token_info);
         return;
     }
 
-    furi_string_reset(temp_str);
-    furi_string_free(temp_str);
+    furi_string_secure_free(temp_str);
 
     bool load_generate_token_scene = false;
     if(plugin_state->current_scene == TotpSceneGenerateToken) {

totp/services/config/config.c

@@ -343,9 +343,9 @@ TokenLoadingResult totp_config_file_load_tokens(PluginState* const plugin_state)
 
         TokenInfo* tokenInfo = token_info_alloc();
 
-        const char* temp_cstr = furi_string_get_cstr(temp_str);
-        tokenInfo->name = (char*)malloc(strlen(temp_cstr) + 1);
-        strcpy(tokenInfo->name, temp_cstr);
+        size_t temp_cstr_len = furi_string_size(temp_str);
+        tokenInfo->name = (char*)malloc(temp_cstr_len + 1);
+        strlcpy(tokenInfo->name, furi_string_get_cstr(temp_str), temp_cstr_len + 1);
 
         uint32_t secret_bytes_count;
         if(!flipper_format_get_value_count(
@@ -355,9 +355,11 @@ TokenLoadingResult totp_config_file_load_tokens(PluginState* const plugin_state)
 
         if(secret_bytes_count == 1) { // Plain secret key
             if(flipper_format_read_string(fff_data_file, TOTP_CONFIG_KEY_TOKEN_SECRET, temp_str)) {
-                temp_cstr = furi_string_get_cstr(temp_str);
                 if(token_info_set_secret(
-                       tokenInfo, temp_cstr, strlen(temp_cstr), &plugin_state->iv[0])) {
+                       tokenInfo,
+                       furi_string_get_cstr(temp_str),
+                       furi_string_size(temp_str),
+                       &plugin_state->iv[0])) {
                     FURI_LOG_W(LOGGING_TAG, "Token \"%s\" has plain secret", tokenInfo->name);
                 } else {
                     tokenInfo->token = NULL;