FWEO-1481 app_strorage: Add CRC check at initialization + unit tests

Force reset in case the storage is corrupted.

Author: jarevalo-ledger

include/decorators.h

@@ -72,3 +72,9 @@
 #else
 #define STATIC
 #endif
+
+#if !defined(UNIT_TESTING)
+#define CONST const
+#else
+#define CONST
+#endif

lib_standard_app/app_storage.c

@@ -24,25 +24,34 @@
 
 #define APP_STORAGE_ERASE_BLOCK_SIZE 256
 
-/* In order to be used in unit testing */
-#if !defined(TEST)
-#define CONST const
-#else
-#define CONST
-#endif
-
 CONST app_storage_t app_storage_real __attribute__((section(".storage_section")));
 #define app_storage (*(volatile app_storage_t *) PIC(&app_storage_real))
 
 /**
- * @brief checks if the app storage struct is initialized
+ * @brief checks if the app storage struct is initialized and valid
  */
-static bool app_storage_is_initalized(void)
+STATIC bool app_storage_is_initalized(void)
 {
-    if (memcmp((const void *) &app_storage.header.tag, APP_STORAGE_TAG, APP_STORAGE_TAG_LEN) != 0) {
-        return false;
+    bool is_initialized = false;
+    if (memcmp((const void *) &app_storage.header.tag, APP_STORAGE_TAG, APP_STORAGE_TAG_LEN) == 0) {
+        is_initialized = true;
+    }
+    else {
+        goto error;
+    }
+
+    uint32_t crc = cx_crc32((void *) &app_storage.header,
+                            sizeof(app_storage.header) + app_storage.header.size);
+    if (crc != app_storage.crc) {
+        // Invalid CRC, force reset
+        is_initialized = false;
     }
-    return true;
+    else {
+        is_initialized = true;
+    }
+
+error:
+    return is_initialized;
 }
 
 static inline void update_crc(void)

unit-tests/app_storage/CMakeLists.txt

@@ -66,7 +66,7 @@ target_compile_options(
 target_compile_definitions(
   ${TEST}
   PRIVATE
-  TEST
+  UNIT_TESTING
   SCREEN_SIZE_WALLET
   USB_SEGMENT_SIZE=64
   HAVE_NBGL

unit-tests/app_storage/app_storage_stubs.c

@@ -18,6 +18,16 @@
 #include <setjmp.h>
 #include <string.h>
 
+#ifdef UNIT_TESTING
+// When defined cmocka redefine malloc/free which does not work well with
+// address-sanitizer
+#undef UNIT_TESTING
+#include <cmocka.h>
+#define UNIT_TESTING
+#else
+#include <cmocka.h>
+#endif
+
 #include <cmocka.h>
 #include <malloc.h>
 #include <stdio.h>

unit-tests/app_storage/test_app_storage.c

@@ -17,14 +17,23 @@
 #include <stddef.h>
 #include <setjmp.h>
 #include <string.h>
-
-#include <cmocka.h>
 #include <stdio.h>
 #include <stdlib.h>
 
+#ifdef UNIT_TESTING
+// When defined cmocka redefine malloc/free which does not work well with
+// address-sanitizer
+#undef UNIT_TESTING
+#include <cmocka.h>
+#define UNIT_TESTING
+#else
+#include <cmocka.h>
+#endif
+
 #include "app_storage.h"
 #include "app_storage_internal.h"
 #include "app_storage_stubs.h"
+#include "os_nvm.h"
 
 /* Defines */
 #define INITIAL_SIZE      20
@@ -68,6 +77,10 @@ _Static_assert(sizeof(app_storage_data_t) <= APP_STORAGE_SIZE,
     app_storage_read(                      \
         dst_buf, sizeof(((app_storage_data_t *) 0)->field), offsetof(app_storage_data_t, field))
 
+// app_storage.h private
+extern app_storage_t app_storage_real;
+bool                 app_storage_is_initalized(void);
+
 /* Local prototypes */
 static void test_write_read_from_empty(void **state __attribute__((unused)));
 static void test_app_style_from_empty(void **state __attribute__((unused)));
@@ -119,6 +132,55 @@ static void test_getters_from_empty(void **state __attribute__((unused)))
                      APP_STORAGE_PROP_SETTINGS | APP_STORAGE_PROP_DATA);
 }
 
+/* Test that corruption from empty storage is detected */
+static void test_corrupted_storage_from_empty(void **state __attribute__((unused)))
+{
+    assert_true(app_storage_is_initalized());
+    // --- Simulate corrupted header
+    app_storage_header_t header = app_storage_real.header;
+    header.data_version += 1;
+    // Change header with no CRC update
+    nvm_write((void *) &app_storage_real.header, &header, sizeof(header));
+    // Ensure invalid CRC
+    assert_false(app_storage_is_initalized());
+
+    // --- Simulate corrupted data
+    setup_from_empty(NULL);
+    assert_true(app_storage_is_initalized());
+    uint8_t buf[20] = {0};
+    memset(buf, 0xAA, sizeof(buf));
+    assert_int_equal(app_storage_write(buf, sizeof(buf), 0), sizeof(buf));
+    // Change data with no CRC update
+    buf[sizeof(buf) - 1] = 0xAB;
+    nvm_write((void *) &app_storage_real.data, buf, sizeof(buf));
+    // Ensure invalid CRC
+    assert_false(app_storage_is_initalized());
+}
+
+/* Test that corruption from prepared storage is detected */
+static void test_corrupted_storage_from_prepared(void **state __attribute__((unused)))
+{
+    assert_true(app_storage_is_initalized());
+    // --- Simulate corrupted header
+    app_storage_header_t header = app_storage_real.header;
+    header.data_version += 1;
+    // Change header with no CRC update
+    nvm_write((void *) &app_storage_real.header, &header, sizeof(header));
+    // Ensure invalid CRC
+    assert_false(app_storage_is_initalized());
+
+    // --- Simulate corrupted data
+    setup_from_prepared(NULL);
+    assert_true(app_storage_is_initalized());
+    uint8_t data[INITIAL_SIZE + ADDITIONALL_SIZE] = {0};
+    app_storage_read(data, INITIAL_SIZE + ADDITIONALL_SIZE, 0);
+    // Change data with no CRC update
+    data[INITIAL_SIZE + ADDITIONALL_SIZE - 1]++;
+    nvm_write((void *) &app_storage_real.data, data, INITIAL_SIZE + ADDITIONALL_SIZE);
+    // Ensure invalid CRC
+    assert_false(app_storage_is_initalized());
+}
+
 /* Read error cases with initially empty storage */
 static void test_read_error_from_empty(void **state __attribute__((unused)))
 {
@@ -448,6 +510,8 @@ int main(int argc, char **argv)
 {
     const struct CMUnitTest tests[] = {
         cmocka_unit_test_setup_teardown(test_getters_from_empty, setup_from_empty, teardown),
+        cmocka_unit_test_setup_teardown(
+            test_corrupted_storage_from_empty, setup_from_empty, teardown),
         cmocka_unit_test_setup_teardown(test_read_error_from_empty, setup_from_empty, teardown),
         cmocka_unit_test_setup_teardown(test_write_error_from_empty, setup_from_empty, teardown),
         cmocka_unit_test_setup_teardown(test_data_version_from_empty, setup_from_empty, teardown),
@@ -456,6 +520,8 @@ int main(int argc, char **argv)
             test_write_big_reset_from_empty, setup_from_empty, teardown),
         cmocka_unit_test_setup_teardown(
             test_write_read_from_prepared, setup_from_prepared, teardown),
+        cmocka_unit_test_setup_teardown(
+            test_corrupted_storage_from_prepared, setup_from_prepared, teardown),
         cmocka_unit_test_setup_teardown(test_app_style_from_empty, setup_from_empty, teardown),
         cmocka_unit_test_setup_teardown(
             test_app_style_from_prepared, setup_from_prepared_app_style, teardown),