Public commit record
98/100 · Strong
What the developer wrote
Authored by benma's agent
build: enable -Wcast-align=strict
Enable GCC -Wcast-align=strict globally (Clang uses -Wcast-align) so
misaligned pointer-cast issues are caught across firmware, bootloader,
unit tests, and simulators.
Fixes fall into two UB classes:
1) Alignment UB: casting byte buffers/packed payloads to wider pointer
types and then dereferencing can require stricter alignment than the
source object provides. This is undefined behavior per C.
2) Effective-type / strict-aliasing UB: reading a value by
reinterpreting a uint8_t buffer as a different object type via a
pointer cast violates C’s effective-type/aliasing rules, so the
compiler may miscompile even when the address happens to be aligned.
Resolve by memcpy’ing into
properly-typed locals (e.g. USB_FRAME, u32/version_t) before use.
Useful references:
- https://www.open-std.org/jtc1/sc22/WG14/www/docs/n3519.pdf
For C11 (ISO/IEC 9899:2011), the key places are:
- Alignment UB from pointer casts: §6.3.2.3 p7 (“Pointers”) — converting to a different object pointer type and the result not being correctly aligned is UB.
- Effective type rule: §6.5 p6 (“Expressions”) — defines an object’s effective type (including the memcpy/memmove wording).
- Strict-aliasing rule: §6.5 p7 — lists the allowed lvalue types you may use to access an object’s stored value (the classic aliasing bullet list).
✓ Descriptive subject✓ Names a concrete action or component✓ Uses a recognizable type or scope✓ Provides detailed explanatory context✓ Mentions testing or verification✓ Links an issue, advisory, or supporting reference
What changed, and why it matters
This commit turns on a stricter compiler warning that catches risky pointer casts in C code, and fixes the resulting warnings. The changes replace direct casts from byte buffers to larger types (like treating a raw byte array as a 32-bit number or USB frame) with safer memcpy-based copies. The commit message explicitly frames these as classes of undefined behavior (alignment and strict-aliasing issues) that could lead to miscompilation or incorrect reads. It is a hardening/correctness patch rather than a fix for a known exploitable bug, but the affected code paths include bootloader pairing-code handling, firmware version parsing, USB packet processing, and BLE pairing-code display.
Recommended action
Treat as a proactive hardening and code-correctness improvement. Review that all new memcpy destinations are no larger than the source buffers and that no out-of-bounds reads were introduced. Consider whether any of the replaced casts were reachable from attacker-controlled input (USB/BLE/RTT/firmware metadata) and whether additional static analysis or fuzzing is warranted for those paths. No urgent patch deployment is indicated absent a disclosed exploit.
Security signals we found
01
Undefined behavior remediation: alignment UB from unaligned pointer casts
02
Undefined behavior remediation: strict-aliasing / effective-type violations
03
Compiler hardening: enabling -Wcast-align=strict globally
04
Affected paths include bootloader pairing code derivation, firmware version parsing, USB/BLE packet processing, factory setup RTT length read
05
No explicit CVE, advisory, or exploit disclosure referenced in commit
Evidence from the diff
The patch enables -Wcast-align=strict (GCC) / -Wcast-align (Clang) globally and suppresses it only for third-party/external code (asf4-drivers-min, u2f-util). It then replaces pointer-cast dereferences with memcpy into properly typed locals in bootloader.c, startup.c, da14531_handler.c, factorysetup.c, firmware_main_loop.c, simulator.c, mock_hidapi.c, and test_memory.c. The two UB classes cited are (1) alignment UB from converting uint8_t* to wider types and dereferencing, and (2) effective-type/strict-aliasing UB from accessing a uint8_t buffer through a different object type. The patch does not change wire formats or behavior when inputs are well-formed; it only changes how data is loaded into typed variables.
Changed components
CMakeLists.txt build flagssrc/bootloader/bootloader.csrc/bootloader/startup.csrc/da14531/da14531_handler.csrc/factorysetup.csrc/firmware_main_loop.ctest/simulator/simulator.ctest/unit-test/framework/src/mock_hidapi.ctest/unit-test/test_memory.cexternal/CMakeLists.txt (asf4-drivers-min warning suppression)test/unit-test/CMakeLists.txt (u2f-util warning suppression)Inspect captured patch +43 / −24
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 43dc63a..2fc352c 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -299,6 +299,13 @@ string(APPEND CMAKE_C_FLAGS " -Wold-style-definition -Wswitch-default -Wattribut
string(APPEND CMAKE_C_FLAGS " -Wdeprecated-declarations -Wcast-qual -Wstrict-prototypes")
string(APPEND CMAKE_C_FLAGS " -Wundef -Wmissing-include-dirs")
+# Strict alignment checks for pointer casts (GCC only).
+if(CMAKE_C_COMPILER_ID STREQUAL "GNU")
+ string(APPEND CMAKE_C_FLAGS " -Wcast-align=strict")
+elseif(CMAKE_C_COMPILER_ID MATCHES "Clang")
+ string(APPEND CMAKE_C_FLAGS " -Wcast-align")
+endif()
+
# Disable builtin warning
string(APPEND CMAKE_C_FLAGS " -Wno-cast-function-type")
diff --git a/external/CMakeLists.txt b/external/CMakeLists.txt
index c8ab86d..be7370a 100644
--- a/external/CMakeLists.txt
+++ b/external/CMakeLists.txt
@@ -73,7 +73,7 @@ if(CMAKE_CROSSCOMPILING)
asf4-drivers/usb/device/usbdc.c
)
- target_compile_options(asf4-drivers-min PRIVATE -Wno-cast-qual -Wno-unused-parameter -Wno-missing-prototypes -Wno-missing-declarations -Wno-bad-function-cast -Wno-strict-prototypes -Wno-old-style-definition)
+ target_compile_options(asf4-drivers-min PRIVATE -Wno-cast-qual -Wno-unused-parameter -Wno-missing-prototypes -Wno-missing-declarations -Wno-bad-function-cast -Wno-strict-prototypes -Wno-old-style-definition -Wno-cast-align)
if (CMAKE_BUILD_TYPE STREQUAL "DEBUG")
target_compile_definitions(asf4-drivers-min PUBLIC DEBUG USE_SIMPLE_ASSERT)
@@ -82,7 +82,7 @@ if(CMAKE_CROSSCOMPILING)
target_link_libraries(asf4-drivers-min samd51a-ds)
set_property(TARGET asf4-drivers-min PROPERTY INTERFACE_LINK_LIBRARIES "")
target_compile_options(asf4-drivers-min PRIVATE -Wno-cast-qual
- -Wno-unused-parameter -Wno-missing-prototypes -Wno-missing-declarations)
+ -Wno-unused-parameter -Wno-missing-prototypes -Wno-missing-declarations -Wno-cast-align)
target_include_directories(asf4-drivers-min SYSTEM
PUBLIC
diff --git a/src/bootloader/bootloader.c b/src/bootloader/bootloader.c
index 23b18ba..82eb4bd 100644
--- a/src/bootloader/bootloader.c
+++ b/src/bootloader/bootloader.c
@@ -353,7 +353,9 @@ void bootloader_render_ble_confirm_screen(bool confirmed)
{
qtouch_force_calibrate();
bootloader_pairing_request = true;
- uint32_t pairing_code_int = (*(uint32_t*)&bootloader_pairing_code_bytes[0]) % 1000000;
+ uint32_t pairing_code_int;
+ memcpy(&pairing_code_int, &bootloader_pairing_code_bytes[0], sizeof(pairing_code_int));
+ pairing_code_int %= 1000000;
char code_str[10] = {0};
snprintf(code_str, sizeof(code_str), "%06u", (unsigned)pairing_code_int);
UG_ClearBuffer();
@@ -556,7 +558,9 @@ static size_t _api_firmware_erase(uint8_t firmware_num_chunks, uint8_t* output)
static inline version_t _parse_version(const uint8_t* start)
{
// 4 byte little endian
- return *(const version_t*)start;
+ version_t version;
+ memcpy(&version, start, sizeof(version));
+ return version;
}
static void _double_hash(const uint8_t* data, uint32_t len, uint8_t* hash)
diff --git a/src/bootloader/startup.c b/src/bootloader/startup.c
index fb39534..bb5e59a 100644
--- a/src/bootloader/startup.c
+++ b/src/bootloader/startup.c
@@ -78,7 +78,7 @@ int main(void)
// If did not jump to firmware code, begin UART/USB processing
const uint8_t* hww_data = NULL;
- uint8_t hww_frame[USB_REPORT_SIZE] = {0};
+ USB_FRAME hww_frame = {0};
#if PLATFORM_BITBOX02PLUS == 1
uint8_t uart_read_buf[USART_0_BUFFER_SIZE] = {0};
@@ -128,8 +128,8 @@ int main(void)
if (!hww_data) {
hww_data = queue_pull(queue_hww_queue());
}
- if (!hww_data && hid_hww_read(&hww_frame[0])) {
- usb_packet_process((const USB_FRAME*)hww_frame);
+ if (!hww_data && hid_hww_read((uint8_t*)&hww_frame)) {
+ usb_packet_process(&hww_frame);
#if PLATFORM_BITBOX02PLUS == 1
if (rust_communication_mode_ble_enabled()) {
// Enqueue a power down command to the da14531
diff --git a/src/da14531/da14531_handler.c b/src/da14531/da14531_handler.c
index 3d1f4fe..c15e49a 100644
--- a/src/da14531/da14531_handler.c
+++ b/src/da14531/da14531_handler.c
@@ -148,7 +148,9 @@ static void _ctrl_handler(struct da14531_ctrl_frame* frame, struct ringbuffer* q
&frame->cmd_data[0],
sizeof(_ble_pairing_callback_data.key));
_ble_pairing_callback_data.queue = queue;
- uint32_t pairing_code_int = (*(uint32_t*)&frame->cmd_data[0]) % 1000000;
+ uint32_t pairing_code_int;
+ memcpy(&pairing_code_int, &frame->cmd_data[0], sizeof(pairing_code_int));
+ pairing_code_int %= 1000000;
char pairing_code[7] = {0};
snprintf(pairing_code, sizeof(pairing_code), "%06lu", (long unsigned int)pairing_code_int);
// util_log("da14531: show/confirm pairing code: %s", pairing_code);
@@ -273,7 +275,9 @@ static void _hww_handler(struct da14531_protocol_frame* frame, struct ringbuffer
util_log("da14531: invalid hww payload length %u, dropped frame", frame->payload_length);
return;
}
- usb_packet_process((USB_FRAME*)&frame->payload[0]);
+ USB_FRAME usb_frame;
+ memcpy(&usb_frame, &frame->payload[0], sizeof(usb_frame));
+ usb_packet_process(&usb_frame);
}
// Handler must not use the frame pointer after it has returned
diff --git a/src/factorysetup.c b/src/factorysetup.c
index 1becf41..a07399b 100644
--- a/src/factorysetup.c
+++ b/src/factorysetup.c
@@ -274,7 +274,8 @@ static bool _rtt_receive(uint8_t* msg_out, size_t* len_out)
return false;
}
// util_log("read %s", util_dbg_hex(buffer, read));
- uint16_t len = *((uint16_t*)buffer);
+ uint16_t len;
+ memcpy(&len, buffer, sizeof(len));
if (len >= BUFFER_SIZE_DOWN - LENSIZE) {
screen_sprintf_debug(
2000, "Error: read more than buffer size: %d bytes (total read: %d)", len, read);
diff --git a/src/firmware_main_loop.c b/src/firmware_main_loop.c
index bc57cfe..64967d7 100644
--- a/src/firmware_main_loop.c
+++ b/src/firmware_main_loop.c
@@ -74,12 +74,12 @@ void firmware_main_loop(void)
rust_workflow_spawn_orientation_screen();
const uint8_t* hww_data = NULL;
- uint8_t hww_frame[USB_REPORT_SIZE] = {0};
+ USB_FRAME hww_frame = {0};
#if APP_U2F == 1
u2f_packet_init();
const uint8_t* u2f_data = NULL;
- uint8_t u2f_frame[USB_REPORT_SIZE] = {0};
+ USB_FRAME u2f_frame = {0};
#endif
if (!memory_ble_enabled()) {
@@ -119,8 +119,8 @@ void firmware_main_loop(void)
}
#endif
// Do USB Input
- if (!hww_data && hid_hww_read(&hww_frame[0])) {
- if (usb_packet_process((const USB_FRAME*)hww_frame)) {
+ if (!hww_data && hid_hww_read((uint8_t*)&hww_frame)) {
+ if (usb_packet_process(&hww_frame)) {
if (rust_communication_mode_ble_enabled()) {
// Enqueue a power down command to the da14531
da14531_power_down(&uart_write_queue);
@@ -136,9 +136,9 @@ void firmware_main_loop(void)
}
}
#if APP_U2F == 1
- if (!u2f_data && hid_u2f_read(&u2f_frame[0])) {
- util_log("u2f data %s", util_dbg_hex((void*)u2f_frame, 16));
- u2f_packet_process((const USB_FRAME*)u2f_frame);
+ if (!u2f_data && hid_u2f_read((uint8_t*)&u2f_frame)) {
+ util_log("u2f data %s", util_dbg_hex((void*)&u2f_frame, 16));
+ u2f_packet_process(&u2f_frame);
}
#endif
diff --git a/test/simulator/simulator.c b/test/simulator/simulator.c
index 519c5c8..1d7a300 100644
--- a/test/simulator/simulator.c
+++ b/test/simulator/simulator.c
@@ -54,7 +54,9 @@ static void send_usb_message_socket(void)
static void simulate_firmware_execution(const uint8_t* input)
{
- usb_packet_process((const USB_FRAME*)input);
+ USB_FRAME frame;
+ memcpy(&frame, input, sizeof(frame));
+ usb_packet_process(&frame);
rust_workflow_spin();
rust_async_usb_spin();
usb_processing_process(usb_processing_hww());
diff --git a/test/unit-test/CMakeLists.txt b/test/unit-test/CMakeLists.txt
index da824c4..06fe5d7 100644
--- a/test/unit-test/CMakeLists.txt
+++ b/test/unit-test/CMakeLists.txt
@@ -110,6 +110,7 @@ add_library(u2f-util
u2f/uECC.c
u2f/u2f_util_t.c
)
+target_compile_options(u2f-util PRIVATE -Wno-cast-align)
target_include_directories(u2f-util
SYSTEM PUBLIC
${HIDAPI_INCLUDE_DIRS}
diff --git a/test/unit-test/framework/src/mock_hidapi.c b/test/unit-test/framework/src/mock_hidapi.c
index 4841235..d05d545 100644
--- a/test/unit-test/framework/src/mock_hidapi.c
+++ b/test/unit-test/framework/src/mock_hidapi.c
@@ -20,7 +20,7 @@
#define BUFSIZE 0x40LU
-static uint8_t _buf[BUFSIZE];
+static USB_FRAME _buf;
static size_t _buf_len;
static bool _expect_more;
@@ -102,9 +102,9 @@ int hid_write(hid_device* dev, const unsigned char* data, size_t length)
printf("Internal test error: %lu > %lu\n", length - 1, BUFSIZE);
return 0;
}
- memcpy(_buf, data + 1, length - 1);
+ memcpy(&_buf, data + 1, length - 1);
_buf_len = length - 1;
- _expect_more = u2f_packet_process((const USB_FRAME*)_buf);
+ _expect_more = u2f_packet_process(&_buf);
if (!_expect_more) {
// printf("Got complete packet\n");
_have_data = true;
diff --git a/test/unit-test/test_memory.c b/test/unit-test/test_memory.c
index b71287c..9dcf084 100644
--- a/test/unit-test/test_memory.c
+++ b/test/unit-test/test_memory.c
@@ -538,12 +538,12 @@ static void _test_memory_set_seed_birthdate(void** state)
will_return(__wrap_memory_read_chunk_fake, empty_chunk);
EMPTYCHUNK(expected_chunk);
- uint32_t* timestamp = (uint32_t*)&expected_chunk[_addr_seed_birthdate];
- *timestamp = 0xabcdef11;
+ uint32_t timestamp = 0xabcdef11;
+ memcpy(&expected_chunk[_addr_seed_birthdate], ×tamp, sizeof(timestamp));
expect_value(__wrap_memory_write_chunk_fake, chunk_num, 1);
expect_memory(__wrap_memory_write_chunk_fake, chunk, expected_chunk, CHUNK_SIZE);
will_return(__wrap_memory_write_chunk_fake, true);
- assert_true(memory_set_seed_birthdate(*timestamp));
+ assert_true(memory_set_seed_birthdate(timestamp));
}
static void _test_memory_set_attestation_device_pubkey(void** state)
Risk score
Our methodology →Why this scored 53/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.