Fixing SONAR issues (#13)

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

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);
     }
 }

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;
 

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;
 

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);
 }

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);

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;

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")

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) {

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;

services/crypto/crypto.c

@@ -3,6 +3,7 @@
 #include <furi_hal.h>
 #include "../config/config.h"
 #include "../../types/common.h"
+#include "memset_s.h"
 
 #define CRYPTO_KEY_SLOT 2
 #define CRYPTO_VERIFY_KEY "FFF_Crypto_pass"
@@ -11,13 +12,13 @@
 
 uint8_t* totp_crypto_encrypt(
     const uint8_t* plain_data,
-    const uint8_t plain_data_length,
+    const size_t plain_data_length,
     const uint8_t* iv,
-    uint8_t* encrypted_data_length) {
+    size_t* encrypted_data_length) {
     uint8_t* encrypted_data;
     size_t remain = plain_data_length % CRYPTO_ALIGNMENT_FACTOR;
     if(remain) {
-        uint8_t plain_data_aligned_length = plain_data_length - remain + CRYPTO_ALIGNMENT_FACTOR;
+        size_t plain_data_aligned_length = plain_data_length - remain + CRYPTO_ALIGNMENT_FACTOR;
         uint8_t* plain_data_aligned = malloc(plain_data_aligned_length);
         memset(plain_data_aligned, 0, plain_data_aligned_length);
         memcpy(plain_data_aligned, plain_data, plain_data_length);
@@ -29,7 +30,7 @@ uint8_t* totp_crypto_encrypt(
         furi_hal_crypto_encrypt(plain_data_aligned, encrypted_data, plain_data_aligned_length);
         furi_hal_crypto_store_unload_key(CRYPTO_KEY_SLOT);
 
-        memset(plain_data_aligned, 0, plain_data_aligned_length);
+        memset_s(plain_data_aligned, sizeof(plain_data_aligned), 0, plain_data_aligned_length);
         free(plain_data_aligned);
     } else {
         encrypted_data = malloc(plain_data_length);
@@ -45,9 +46,9 @@ uint8_t* totp_crypto_encrypt(
 
 uint8_t* totp_crypto_decrypt(
     const uint8_t* encrypted_data,
-    const uint8_t encrypted_data_length,
+    const size_t encrypted_data_length,
     const uint8_t* iv,
-    uint8_t* decrypted_data_length) {
+    size_t* decrypted_data_length) {
     *decrypted_data_length = encrypted_data_length;
     uint8_t* decrypted_data = malloc(*decrypted_data_length);
     furi_hal_crypto_store_load_key(CRYPTO_KEY_SLOT, iv);
@@ -118,7 +119,7 @@ void totp_crypto_seed_iv(PluginState* plugin_state, uint8_t* pin, uint8_t pin_le
 }
 
 bool totp_crypto_verify_key(const PluginState* plugin_state) {
-    uint8_t decrypted_key_length;
+    size_t decrypted_key_length;
     uint8_t* decrypted_key = totp_crypto_decrypt(
         plugin_state->crypto_verify_data,
         plugin_state->crypto_verify_data_length,

services/crypto/crypto.h

@@ -4,13 +4,13 @@
 
 uint8_t* totp_crypto_encrypt(
     const uint8_t* plain_data,
-    const uint8_t plain_data_length,
+    const size_t plain_data_length,
     const uint8_t* iv,
-    uint8_t* encrypted_data_length);
+    size_t* encrypted_data_length);
 uint8_t* totp_crypto_decrypt(
     const uint8_t* encrypted_data,
-    const uint8_t encrypted_data_length,
+    const size_t encrypted_data_length,
     const uint8_t* iv,
-    uint8_t* decrypted_data_length);
+    size_t* decrypted_data_length);
 void totp_crypto_seed_iv(PluginState* plugin_state, uint8_t* pin, uint8_t pin_length);
 bool totp_crypto_verify_key(const PluginState* plugin_state);

services/crypto/memset_s.c

@@ -0,0 +1,22 @@
+#include "memset_s.h"
+
+#define RSIZE_MAX 0x7fffffffffffffffUL
+
+errno_t memset_s(void* s, rsize_t smax, int c, rsize_t n) {
+    if(!s || smax > RSIZE_MAX) {
+        return EINVAL;
+    }
+
+    errno_t violation_present = 0;
+    if(n > smax) {
+        n = smax;
+        violation_present = EINVAL;
+    }
+
+    volatile unsigned char* v = s;
+    for(rsize_t i = 0u; i < n; ++i) {
+        *v++ = (unsigned char)c;
+    }
+
+    return violation_present;
+}

services/crypto/memset_s.h

@@ -0,0 +1,16 @@
+#pragma once
+
+#include <errno.h>
+#include <stdint.h>
+#include <string.h>
+
+#ifndef _RSIZE_T_DECLARED
+typedef uint64_t rsize_t;
+#define _RSIZE_T_DECLARED
+#endif
+#ifndef _ERRNOT_DECLARED
+typedef int16_t errno_t;
+#define _ERRNOT_DECLARED
+#endif
+
+errno_t memset_s(void* s, rsize_t smax, int c, rsize_t n);

services/totp/totp.c

@@ -42,7 +42,7 @@ uint32_t otp_generate(
     TOTP_ALGO algo,
     uint8_t digits,
     const uint8_t* plain_secret,
-    uint8_t plain_secret_length,
+    size_t plain_secret_length,
     uint64_t input) {
     uint8_t* hmac = malloc(64);
     memset(hmac, 0, 64);
@@ -80,7 +80,7 @@ uint32_t totp_at(
     TOTP_ALGO algo,
     uint8_t digits,
     const uint8_t* plain_secret,
-    uint8_t plain_secret_length,
+    size_t plain_secret_length,
     uint64_t for_time,
     float timezone,
     uint8_t interval) {
@@ -96,9 +96,9 @@ uint32_t totp_at(
 
 static int totp_algo_sha1(
     const uint8_t* key,
-    uint8_t key_length,
+    size_t key_length,
     const uint8_t* input,
-    uint8_t input_length,
+    size_t input_length,
     uint8_t* output) {
     hmac_sha1(key, key_length, input, input_length, output);
     return HMAC_SHA1_RESULT_SIZE;
@@ -106,9 +106,9 @@ static int totp_algo_sha1(
 
 static int totp_algo_sha256(
     const uint8_t* key,
-    uint8_t key_length,
+    size_t key_length,
     const uint8_t* input,
-    uint8_t input_length,
+    size_t input_length,
     uint8_t* output) {
     hmac_sha256(key, key_length, input, input_length, output);
     return HMAC_SHA256_RESULT_SIZE;
@@ -116,9 +116,9 @@ static int totp_algo_sha256(
 
 static int totp_algo_sha512(
     const uint8_t* key,
-    uint8_t key_length,
+    size_t key_length,
     const uint8_t* input,
-    uint8_t input_length,
+    size_t input_length,
     uint8_t* output) {
     hmac_sha512(key, key_length, input, input_length, output);
     return HMAC_SHA512_RESULT_SIZE;

services/totp/totp.h

@@ -17,9 +17,9 @@
 */
 typedef int (*TOTP_ALGO)(
     const uint8_t* key,
-    uint8_t key_length,
+    size_t key_length,
     const uint8_t* input,
-    uint8_t input_length,
+    size_t input_length,
     uint8_t* output);
 
 /*
@@ -47,7 +47,7 @@ uint32_t totp_at(
     TOTP_ALGO algo,
     uint8_t digits,
     const uint8_t* plain_secret,
-    uint8_t plain_secret_length,
+    size_t plain_secret_length,
     uint64_t for_time,
     float timezone,
     uint8_t interval);

types/plugin_state.h

@@ -22,7 +22,7 @@ typedef struct {
     uint8_t tokens_count;
 
     uint8_t* crypto_verify_data;
-    uint8_t crypto_verify_data_length;
+    size_t crypto_verify_data_length;
     bool pin_set;
     uint8_t iv[TOTP_IV_SIZE];
     uint8_t base_iv[TOTP_IV_SIZE];

types/token_info.c

@@ -5,6 +5,7 @@
 #include "common.h"
 #include "../services/base32/base32.h"
 #include "../services/crypto/crypto.h"
+#include "../services/crypto/memset_s.h"
 
 TokenInfo* token_info_alloc() {
     TokenInfo* tokenInfo = malloc(sizeof(TokenInfo));
@@ -23,7 +24,7 @@ void token_info_free(TokenInfo* token_info) {
 bool token_info_set_secret(
     TokenInfo* token_info,
     const char* base32_token_secret,
-    uint8_t token_secret_length,
+    size_t token_secret_length,
     uint8_t* iv) {
     uint8_t* plain_secret = malloc(token_secret_length);
     int plain_secret_length =
@@ -37,7 +38,7 @@ bool token_info_set_secret(
         result = false;
     }
 
-    memset(plain_secret, 0, token_secret_length);
+    memset_s(plain_secret, sizeof(plain_secret), 0, token_secret_length);
     free(plain_secret);
     return result;
 }

types/token_info.h

@@ -8,7 +8,7 @@ typedef enum { TOTP_6_DIGITS, TOTP_8_DIGITS } TokenDigitsCount;
 
 typedef struct {
     uint8_t* token;
-    uint8_t token_length;
+    size_t token_length;
     char* name;
     TokenHashAlgo algo;
     TokenDigitsCount digits;
@@ -19,6 +19,6 @@ void token_info_free(TokenInfo* token_info);
 bool token_info_set_secret(
     TokenInfo* token_info,
     const char* base32_token_secret,
-    uint8_t token_secret_length,
+    size_t token_secret_length,
     uint8_t* iv);
 uint8_t token_info_get_digits_count(TokenInfo* token_info);