memory: validate name when setting/getting

While the BitBox02 API memory_setup() and SetDeviceName API checks
should make sure that `memory_get_device_name()` always returns a
valid ASCII-encoded string, we add explicit checks to make sure it is
so.

The reason for this is that the name is sent over to the Bluetooth
firmware in the BB02+, and an invalid string there could cause a
crash.

Author: benma

src/memory/memory.c

@@ -250,6 +250,10 @@ bool memory_set_device_name(const char* name)
     _read_chunk(CHUNK_1, chunk_bytes);
     util_zero(chunk.fields.device_name, sizeof(chunk.fields.device_name));
     snprintf((char*)&chunk.fields.device_name, MEMORY_DEVICE_NAME_MAX_LEN, "%s", name);
+
+    if (!rust_util_is_name_valid(chunk.fields.device_name, MEMORY_DEVICE_NAME_MAX_LEN)) {
+        return false;
+    }
     return _write_chunk(CHUNK_1, chunk.bytes);
 }
 
@@ -286,7 +290,8 @@ void memory_get_device_name(char* name_out)
     chunk_1_t chunk = {0};
     CLEANUP_CHUNK(chunk);
     _read_chunk(CHUNK_1, chunk_bytes);
-    if (chunk.fields.device_name[0] == 0xFF) {
+    if (chunk.fields.device_name[0] == 0xFF ||
+        !rust_util_is_name_valid(chunk.fields.device_name, MEMORY_DEVICE_NAME_MAX_LEN)) {
         if (memory_get_platform() == MEMORY_PLATFORM_BITBOX02_PLUS) {
             // For Bluetooth, we want to use an unambiguous default name so this BitBox can be
             // identified if multiple BitBoxes are advertising at the same time.

src/memory/memory.h

@@ -66,11 +66,14 @@ extern const char* MEMORY_DEFAULT_DEVICE_NAME;
 // memory.c)
 #define MEMORY_DEVICE_NAME_MAX_LEN (64)
 
-// set device name. name is an utf8-encoded string, and null terminated. The max
-// size (including the null terminator) is MEMORY_DEVICE_NAME_MAX_LEN bytes.
+// set device name. name is null terminated. The name must be smaller or equal to
+// MEMORY_DEVICE_NAME_MAX_LEN (including the null terminator) and larger than 0 in size, consist of
+// printable ASCII characters only (and space), not start or end with whitespace, and contain no
+// whitespace other than space.
 USE_RESULT bool memory_set_device_name(const char* name);
 
-// name_out must have MEMORY_DEVICE_NAME_MAX_LEN bytes in size. If no device name is set, we return:
+// name_out must have MEMORY_DEVICE_NAME_MAX_LEN bytes in size. If no device name is set, or if it
+// is invalid, we return:
 // - `MEMORY_DEFAULT_DEVICE_NAME` for non-bluetooth enabled BitBoxes
 // - "BitBox ABCD" for Bluetooth-enabled BitBoxes, where ABCD are four random uppercase letters.
 //    The name is cached in RAM, so the same random name is returned until reboot.

src/rust/bitbox02-rust-c/src/util.rs

@@ -25,6 +25,24 @@ pub extern "C" fn rust_util_zero(mut dst: BytesMut) {
     util::zero(dst.as_mut())
 }
 
+/// Calls `util::name::validate()` on the provided C string.
+/// SAFETY:
+/// `buf` must point to a valid buffer of size `max_len`.
+#[no_mangle]
+pub unsafe extern "C" fn rust_util_is_name_valid(buf: *const u8, max_len: usize) -> bool {
+    if max_len == 0 {
+        return false;
+    }
+    let slice = core::slice::from_raw_parts(buf, max_len);
+    match core::ffi::CStr::from_bytes_until_nul(slice) {
+        Ok(cstr) => match cstr.to_str() {
+            Ok(s) => util::name::validate(s, max_len - 1),
+            Err(_) => false,
+        },
+        Err(_) => false,
+    }
+}
+
 /// Convert bytes to hex representation
 ///
 /// * `buf` - bytes to convert to hex.
@@ -202,4 +220,22 @@ mod tests {
         let expected = b"LUC1eAJa5jW\0";
         assert_eq!(&result_buf[..expected.len()], expected);
     }
+
+    #[test]
+    // For clarity we want explicit null terminators in the strings below, not CStr literals
+    // `c"..."`.
+    #[allow(clippy::manual_c_str_literals)]
+    fn test_rust_util_is_name_valid() {
+        unsafe {
+            // Valid
+            assert!(rust_util_is_name_valid("foo\0".as_ptr(), 4));
+            assert!(rust_util_is_name_valid("foo\0........".as_ptr(), 12));
+
+            // Invalid
+            assert!(!rust_util_is_name_valid("fo\no\0".as_ptr(), 5));
+            assert!(!rust_util_is_name_valid("".as_ptr(), 0));
+            assert!(!rust_util_is_name_valid("foo\0".as_ptr(), 3));
+            assert!(!rust_util_is_name_valid("foo".as_ptr(), 3));
+        }
+    }
 }

test/unit-test/test_memory.c

@@ -423,14 +423,31 @@ static void _test_memory_get_device_name_default_bluetooth(void** state)
     assert_string_equal("BitBox AZUV", name_out);
 }
 
-static void _test_memory_get_device_name(void** state)
+static void _test_memory_get_device_name_invalid(void** state)
 {
     char name_out[MEMORY_DEVICE_NAME_MAX_LEN] = {0};
     EMPTYCHUNK(chunk);
     memset(chunk + _addr_device_name, 0, MEMORY_DEVICE_NAME_MAX_LEN);
     const char* device_name = "Äxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxx 漢字xxxxxxxxxxxxxxxxx";
     snprintf((char*)chunk + _addr_device_name, MEMORY_DEVICE_NAME_MAX_LEN, "%s", device_name);
 
+    EMPTYCHUNK(empty_shared_chunk);
+    will_return(__wrap_memory_read_shared_bootdata_mock, empty_shared_chunk);
+
+    expect_value(__wrap_memory_read_chunk_mock, chunk_num, 1);
+    will_return(__wrap_memory_read_chunk_mock, chunk);
+    memory_get_device_name(name_out);
+    assert_string_equal(MEMORY_DEFAULT_DEVICE_NAME, name_out);
+}
+
+static void _test_memory_get_device_name(void** state)
+{
+    char name_out[MEMORY_DEVICE_NAME_MAX_LEN] = {0};
+    EMPTYCHUNK(chunk);
+    memset(chunk + _addr_device_name, 0, MEMORY_DEVICE_NAME_MAX_LEN);
+    const char* device_name = "foo bar";
+    snprintf((char*)chunk + _addr_device_name, MEMORY_DEVICE_NAME_MAX_LEN, "%s", device_name);
+
     expect_value(__wrap_memory_read_chunk_mock, chunk_num, 1);
     will_return(__wrap_memory_read_chunk_mock, chunk);
     memory_get_device_name(name_out);
@@ -561,6 +578,7 @@ int main(void)
         cmocka_unit_test(_test_memory_reset_hww),
         cmocka_unit_test(_test_memory_get_device_name_default),
         cmocka_unit_test(_test_memory_get_device_name_default_bluetooth),
+        cmocka_unit_test(_test_memory_get_device_name_invalid),
         cmocka_unit_test(_test_memory_get_device_name),
         cmocka_unit_test(_test_memory_device_name),
         cmocka_unit_test(_test_memory_set_seed_birthdate),