feat(core): implementation of nRF FW version readout via SMP commands for realiability increase e.g. for cases when the nRF FW application is broken from whatever reason and can't reply to Trezor's FW version check for possible nRF FW update. The alternative way of FW version readout uses the MCUboo
What changed, and why it matters
This commit adds a backup way for the Trezor device to check the version of the wireless nRF chip's firmware. If the normal app-to-app communication fails, the device can now ask the nRF chip's bootloader directly over a different channel. This is meant to make firmware update decisions more reliable, not to fix a known security bug. The change does not appear to introduce an obvious vulnerability, but it does add parsing of data received from the nRF chip and changes how the device decides whether to force an update.
Review the new CBOR parser for robustness against malformed or adversarial SMP responses, ensure all error paths release the SMP receiver, and verify that the version-comparison fallback cannot be abused to suppress or trigger an unwanted nRF firmware update. Treat this as a reliability/feature change rather than an urgent security patch unless further context emerges.
Security signals we found
New CBOR parser added for untrusted SMP response data
Parsing of version string from external nRF device influences update decision
SMP fallback path reboots nRF into bootloader and back during version check
Error path in `get_version_numbers()` releases receiver on send failure but not on response timeout
Force-update fallback remains after three failed attempts
Evidence from the diff
The patch implements an alternative nRF firmware version readout path using MCUboot’s SMP serial recovery protocol. It adds Rust SMP plumbing for an IMAGE_STATE READ command, parses the returned CBOR payload to extract a version string, and exposes the parsed version to C as nrf_app_version_t. The C update logic in nrf_update_required() is refactored: instead of looping on nrf_get_info() and eventually forcing an update, it now first tries the normal SPI info path (hash comparison), then falls back to the SMP version path (version comparison), and only forces an update after three attempts. Several return types were changed from int to bool, and a missing receiver_release() in the error path of get_version_numbers() is notable.
Changed components
core/embed/io/nrf/stm32u5/nrf_update.ccore/embed/rust/rust_smp.hcore/embed/rust/src/smp/api.rscore/embed/rust/src/smp/image_info.rscore/embed/rust/src/smp/mod.rsInspect captured patch +398 / −31
diff --git a/core/embed/io/nrf/stm32u5/nrf_update.c b/core/embed/io/nrf/stm32u5/nrf_update.c
index 1f7c1856..18e325aa 100644
--- a/core/embed/io/nrf/stm32u5/nrf_update.c
+++ b/core/embed/io/nrf/stm32u5/nrf_update.c
@@ -33,13 +33,6 @@
#define IMAGE_HASH_LEN 32
#define IMAGE_TLV_SHA256 0x10
-struct image_version {
- uint8_t iv_major;
- uint8_t iv_minor;
- uint16_t iv_revision;
- uint32_t iv_build_num;
-} __packed;
-
struct image_header {
uint32_t ih_magic;
uint32_t ih_load_addr;
@@ -47,7 +40,7 @@ struct image_header {
uint16_t ih_protect_tlv_size; /* Size of protected TLV area (bytes). */
uint32_t ih_img_size; /* Does not include header. */
uint32_t ih_flags; /* IMAGE_F_[...]. */
- struct image_version ih_ver;
+ nrf_app_version_t ih_ver;
uint32_t _pad1;
} __packed;
@@ -56,11 +49,11 @@ struct image_header {
*
* @param binary_ptr pointer to the binary image
* @param out_hash Buffer of at least IMAGE_HASH_LEN bytes to receive the hash
- * @return 0 on success, or a negative errno on failure
+ * @return "true" on success, "false" on failure
*/
-static int read_image_sha256(const uint8_t *binary_ptr, size_t binary_size,
- uint8_t out_hash[IMAGE_HASH_LEN]) {
- int rc;
+static bool read_image_sha256(const uint8_t *binary_ptr, size_t binary_size,
+ uint8_t out_hash[IMAGE_HASH_LEN]) {
+ bool ret;
/* Read header to get image_size and hdr_size */
struct image_header *hdr = (struct image_header *)binary_ptr;
@@ -77,7 +70,7 @@ static int read_image_sha256(const uint8_t *binary_ptr, size_t binary_size,
uint16_t tlv_hdr[2];
if (off + sizeof(tlv_hdr) > binary_size) {
- rc = -1; // Not enough data for TLV header
+ ret = false; // Not enough data for TLV header
break;
}
@@ -87,16 +80,16 @@ static int read_image_sha256(const uint8_t *binary_ptr, size_t binary_size,
uint16_t len = tlv_hdr[1];
if (off + sizeof(tlv_hdr) + len > binary_size) {
- rc = -1; // Not enough data for TLV value
+ ret = false; // Not enough data for TLV value
break;
}
if (type == IMAGE_TLV_SHA256) {
if (len != IMAGE_HASH_LEN) {
- rc = -1;
+ ret = false;
} else {
memcpy(out_hash, binary_ptr + off + sizeof(tlv_hdr), IMAGE_HASH_LEN);
- rc = 0;
+ ret = true;
}
break;
}
@@ -104,28 +97,101 @@ static int read_image_sha256(const uint8_t *binary_ptr, size_t binary_size,
off += sizeof(tlv_hdr) + len;
}
- return rc;
+ return ret;
+}
+
+/**
+ * Read the image version from the image header.
+ *
+ * @param image_ptr pointer to the binary image
+ * @param out_version Pointer to nrf_app_version_t to receive the version
+ * @return "true" on success, "false" on failure
+ */
+static bool image_version_read(const uint8_t *image_ptr,
+ nrf_app_version_t *out_version) {
+ struct image_header *hdr = (struct image_header *)image_ptr;
+
+ if (image_ptr == NULL || out_version == NULL) {
+ return false;
+ }
+
+ memcpy(out_version, &hdr->ih_ver, sizeof(nrf_app_version_t));
+
+ return true;
+}
+
+/**
+ * Read the image version from the nRF MCUboot via SMP serial recovery.
+ *
+ * @param out_version Pointer to nrf_app_version_t to receive the version
+ * @return "true" on success, "false" on failure
+ */
+static bool nrf_smp_version_get(nrf_app_version_t *out_version) {
+ bool ret = false;
+
+ nrf_reboot_to_bootloader();
+ nrf_set_dfu_mode(true);
+
+ if (smp_image_version_get(out_version)) {
+ // Success - version string provided via SMP has been decoded and stored
+ // within "out_version" variable
+ ret = true;
+ }
+
+ nrf_reboot();
+ nrf_set_dfu_mode(false);
+
+ return ret;
+}
+
+/**
+ * Comparison of two image versions.
+ *
+ * @param v1 Pointer to first nrf_app_version_t
+ * @param v2 Pointer to second nrf_app_version_t
+ * @return 0 when equal, 1 when v1 is greater, -1 when v2 is greater
+ */
+static int version_cmp(const nrf_app_version_t *v1,
+ const nrf_app_version_t *v2) {
+ if (v1->major != v2->major) {
+ return (v1->major < v2->major) ? -1 : 1;
+ }
+ if (v1->minor != v2->minor) {
+ return (v1->minor < v2->minor) ? -1 : 1;
+ }
+ if (v1->revision != v2->revision) {
+ return (v1->revision < v2->revision) ? -1 : 1;
+ }
+ if (v1->build_num != v2->build_num) {
+ return (v1->build_num < v2->build_num) ? -1 : 1;
+ }
+ return 0;
}
bool nrf_update_required(const uint8_t *image_ptr, size_t image_len) {
- nrf_info_t info = {0};
+ for (int i = 0; i < 3; i++) {
+ nrf_info_t info;
+ uint8_t expected_hash[SHA256_DIGEST_LENGTH];
- uint16_t try_cntr = 0;
- while (!nrf_get_info(&info)) {
- nrf_reboot();
- systick_delay_ms(500);
- try_cntr++;
- if (try_cntr > 3) {
- // Assuming corrupted image, but we could also check comm with MCUboot
- return true;
+ if (nrf_get_info(&info) == true &&
+ read_image_sha256(image_ptr, image_len, expected_hash) == true) {
+ return memcmp(info.hash, expected_hash, SHA256_DIGEST_LENGTH) != 0;
}
- }
- uint8_t expected_hash[SHA256_DIGEST_LENGTH] = {0};
+ // Can't communicate with the App via SPI, trying SMP serial recovery over
+ // UART to nRF MCUboot
+ nrf_app_version_t smp_version, image_version;
+
+ if (nrf_smp_version_get(&smp_version) == true &&
+ image_version_read(image_ptr, &image_version) == true) {
+ return version_cmp(&image_version, &smp_version) != 0;
+ }
- read_image_sha256(image_ptr, image_len, expected_hash);
+ systick_delay_ms(100); // TODO: is it necessary?
+ }
- return memcmp(info.hash, expected_hash, SHA256_DIGEST_LENGTH) != 0;
+ // Assuming corrupted image, force update
+ return true;
}
bool nrf_update(const uint8_t *image_ptr, size_t image_len) {
diff --git a/core/embed/rust/rust_smp.h b/core/embed/rust/rust_smp.h
index 0516290d..2bf7eb95 100644
--- a/core/embed/rust/rust_smp.h
+++ b/core/embed/rust/rust_smp.h
@@ -1,12 +1,96 @@
+/*
+ * This file is part of the Trezor project, https://trezor.io/
+ *
+ * Copyright (c) SatoshiLabs
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
#pragma once
#include <trezor_types.h>
+/**
+ * @brief Parsed nRF application version in format
+ * "major.minor.revision[.build_num]".
+ *
+ * Matches MCUboot image header layout (8 bytes total).
+ * If the optional build number is absent it defaults to 0.
+ */
+typedef struct __packed {
+ uint8_t major; /**< Major version (0..255). */
+ uint8_t minor; /**< Minor version (0..255). */
+ uint16_t revision; /**< Revision (0..65535). */
+ uint32_t build_num; /**< Optional build number (0..4294967295). */
+} nrf_app_version_t;
+
+/**
+ * @brief Send an SMP Echo request with a small text payload.
+ *
+ * @param text Pointer to ASCII data (not null-terminated).
+ * @param text_len Number of bytes to send.
+ * @return true on successful send and valid response, false otherwise.
+ *
+ * @note Length is bounded by underlying SMP MTU; oversized input fails.
+ */
bool smp_echo(const char* text, uint8_t text_len);
+/**
+ * @brief Issue an SMP Reset request to the remote nRF device.
+ *
+ * Sends a reset command and does not wait for the device to come back.
+ * @return void
+ */
void smp_reset(void);
+/**
+ * @brief Retrieve and parse the active nRF application version via SMP.
+ *
+ * Performs an Image State read, extracts the version string, and fills
+ * the provided structure with numeric fields.
+ *
+ * @param out Pointer to nrf_app_version_t to be filled (must be valid).
+ * @return true on success, false on failure (communication or parse error).
+ *
+ * @warning Fails if SMP channel not acquired or version format invalid.
+ */
+bool smp_image_version_get(nrf_app_version_t* out);
+
+/**
+ * @brief Feed a received transport byte into the SMP RX state machine.
+ *
+ * Call for each byte arriving from the nRF link (i.e. UART).
+ * Assembles frames and dispatches completed SMP responses internally.
+ *
+ * @param byte Received raw byte.
+ * @return void
+ */
void smp_process_rx_byte(uint8_t byte);
+/**
+ * @brief Upload an MCUboot image to the nRF device over SMP.
+ *
+ * Streams the binary image followed by hash metadata (if required) using
+ * SMP upload semantics.
+ *
+ * @param data Pointer to image buffer.
+ * @param len Size of image buffer in bytes.
+ * @param image_hash Pointer to hash bytes (may be NULL if not used).
+ * @param image_hash_len Length of hash (e.g. 32 for SHA-256).
+ * @return true if upload completed successfully, false on error.
+ *
+ * @note Caller must ensure image fits partition and hash matches expected size.
+ */
bool smp_upload_app_image(const uint8_t* data, size_t len,
const uint8_t* image_hash, size_t image_hash_len);
diff --git a/core/embed/rust/src/smp/api.rs b/core/embed/rust/src/smp/api.rs
index 05ce0f1a..b44ecce2 100644
--- a/core/embed/rust/src/smp/api.rs
+++ b/core/embed/rust/src/smp/api.rs
@@ -1,4 +1,4 @@
-use super::{echo, process_rx_byte, reset, upload};
+use super::{echo, image_info, process_rx_byte, reset, upload};
use crate::util::from_c_array;
@@ -14,6 +14,44 @@ extern "C" fn smp_reset() {
reset::send();
}
+#[repr(C, packed)]
+pub struct NrfAppVersion {
+ pub major: u8,
+ pub minor: u8,
+ pub revision: u16,
+ pub build_num: u32,
+}
+
+/// Get the nRF app version as parsed integer components.
+///
+/// Sends an SMP request to the nRF device to retrieve the active application
+/// image version, then parses the version string into individual numeric
+/// components.
+///
+/// # Arguments
+/// * `out` - Pointer to NrfAppVersion structure to be filled. Must not be NULL.
+///
+/// # Returns
+/// `true` if version was successfully retrieved and parsed, `false` otherwise.
+#[no_mangle]
+extern "C" fn smp_image_version_get(out: *mut NrfAppVersion) -> bool {
+ if out.is_null() {
+ return false;
+ }
+ match image_info::get_version_numbers() {
+ Some(v) => {
+ unsafe {
+ (*out).major = v.major;
+ (*out).minor = v.minor;
+ (*out).revision = v.revision;
+ (*out).build_num = v.build_num;
+ }
+ true
+ }
+ None => false,
+ }
+}
+
#[no_mangle]
extern "C" fn smp_upload_app_image(
data: *const cty::uint8_t,
diff --git a/core/embed/rust/src/smp/image_info.rs b/core/embed/rust/src/smp/image_info.rs
new file mode 100644
index 00000000..27b4ee82
--- /dev/null
+++ b/core/embed/rust/src/smp/image_info.rs
@@ -0,0 +1,173 @@
+use super::{
+ receiver_acquire, receiver_release, send_request, wait_for_response, MsgType, SmpBuffer,
+ SmpHeader, SMP_CMD_ID_IMAGE_STATE, SMP_GROUP_IMAGE, SMP_HEADER_SIZE, SMP_OP_READ,
+};
+use crate::time::Duration;
+use minicbor::{data::Type, decode, Decoder, Encoder};
+
+/// MCUboot-compatible version structure matching image header format
+#[derive(Clone, Copy, Debug)]
+pub struct AppVersion {
+ pub major: u8,
+ pub minor: u8,
+ pub revision: u16,
+ pub build_num: u32,
+}
+
+/// Parse "<major>.<minor>.<revision>[.<build>]" into AppVersion (no allocation)
+/// Matches MCUboot image header version format
+/// Examples: "1.0.3", "1.0.3.0", "255.255.65535.4294967295"
+fn parse_app_version(s: &str) -> Option<AppVersion> {
+ let mut parts = s.split('.');
+
+ let major = parts.next()?.parse::<u8>().ok()?;
+ let minor = parts.next()?.parse::<u8>().ok()?;
+ let revision = parts.next()?.parse::<u16>().ok()?;
+
+ // Build number is optional (defaults to 0 if absent)
+ let build_num = parts
+ .next()
+ .filter(|b| !b.is_empty())
+ .map_or(Some(0), |b| b.parse::<u32>().ok())?;
+
+ // Reject if there are more than 4 parts
+ if parts.next().is_some() {
+ return None;
+ }
+
+ Some(AppVersion {
+ major,
+ minor,
+ revision,
+ build_num,
+ })
+}
+
+/// Send SMP Image State request, parse CBOR, and return parsed version numbers.
+pub fn get_version_numbers() -> Option<AppVersion> {
+ let mut cbor_data = [0u8; 16];
+ let mut data = [0u8; 32];
+ let mut tx_buf = [0u8; 64];
+
+ let mut writer = SmpBuffer::new(&mut cbor_data);
+ let mut enc = Encoder::new(&mut writer);
+ // Empty map as request body
+ unwrap!(enc.map(0));
+
+ unwrap!(receiver_acquire());
+
+ let data_len = writer.bytes_written();
+ let header = SmpHeader::new(
+ SMP_OP_READ,
+ data_len,
+ SMP_GROUP_IMAGE,
+ 0,
+ SMP_CMD_ID_IMAGE_STATE,
+ )
+ .to_bytes();
+
+ data[..SMP_HEADER_SIZE].copy_from_slice(&header);
+ data[SMP_HEADER_SIZE..SMP_HEADER_SIZE + data_len].copy_from_slice(&cbor_data[..data_len]);
+
+ if send_request(&mut data[..SMP_HEADER_SIZE + data_len], &mut tx_buf).is_err() {
+ receiver_release();
+ return None;
+ }
+
+ let mut cbor_payload = [0u8; 256];
+ let res = wait_for_response(
+ MsgType::ImageStateResponse,
+ &mut cbor_payload,
+ Duration::from_millis(2000),
+ );
+ if res.is_err() {
+ return None;
+ }
+
+ extract_version_numbers_from_cbor(&cbor_payload).ok()
+}
+
+// Walk CBOR: { "images": [ { "version": "x.y.z[.t]" , ... } , ... ], ... }
+fn extract_version_numbers_from_cbor(cbor: &[u8]) -> Result<AppVersion, decode::Error> {
+ let mut dec = Decoder::new(cbor);
+
+ // Outer map (definite/indefinite)
+ match dec.map()? {
+ Some(n) => {
+ for _ in 0..n {
+ let key = dec.str()?;
+ if key == "images" {
+ return parse_images_array_for_version(&mut dec);
+ } else {
+ dec.skip()?;
+ }
+ }
+ }
+ None => loop {
+ if let Type::Break = dec.datatype()? {
+ dec.skip()?;
+ break;
+ }
+ let key = dec.str()?;
+ if key == "images" {
+ return parse_images_array_for_version(&mut dec);
+ } else {
+ dec.skip()?;
+ }
+ },
+ }
+
+ Err(decode::Error::message("images not found"))
+}
+
+fn parse_images_array_for_version(dec: &mut Decoder) -> Result<AppVersion, decode::Error> {
+ match dec.array()? {
+ Some(n) => {
+ // Read first element only
+ if n == 0 {
+ return Err(decode::Error::message("no images"));
+ }
+ parse_image_map_for_version(dec)
+ }
+ None => {
+ // Indefinite array: read first item, then stop
+ if let Type::Break = dec.datatype()? {
+ dec.skip()?;
+ return Err(decode::Error::message("no images"));
+ }
+ parse_image_map_for_version(dec)
+ }
+ }
+}
+
+fn parse_image_map_for_version(dec: &mut Decoder) -> Result<AppVersion, decode::Error> {
+ match dec.map()? {
+ Some(n) => {
+ for _ in 0..n {
+ let key = dec.str()?;
+ if key == "version" {
+ let s = dec.str()?;
+ return parse_app_version(s)
+ .ok_or_else(|| decode::Error::message("bad version string"));
+ } else {
+ dec.skip()?;
+ }
+ }
+ }
+ None => loop {
+ if let Type::Break = dec.datatype()? {
+ dec.skip()?;
+ break;
+ }
+ let key = dec.str()?;
+ if key == "version" {
+ let s = dec.str()?;
+ return parse_app_version(s)
+ .ok_or_else(|| decode::Error::message("bad version string"));
+ } else {
+ dec.skip()?;
+ }
+ },
+ }
+ Err(decode::Error::message("version not found"))
+}
diff --git a/core/embed/rust/src/smp/mod.rs b/core/embed/rust/src/smp/mod.rs
index 471e254c..cb8dc42e 100644
--- a/core/embed/rust/src/smp/mod.rs
+++ b/core/embed/rust/src/smp/mod.rs
@@ -2,6 +2,7 @@ mod api;
mod base64;
mod crc16;
mod echo;
+mod image_info;
mod reset;
mod upload;
@@ -24,6 +25,7 @@ pub const SMP_GROUP_IMAGE: u16 = 1;
pub const SMP_CMD_ID_ECHO: u8 = 0;
pub const SMP_CMD_ID_RESET: u8 = 5;
+pub const SMP_CMD_ID_IMAGE_STATE: u8 = 0;
pub const SMP_CMD_ID_IMAGE_UPLOAD: u8 = 1;
pub const SMP_OP_READ: u8 = 0;
@@ -226,6 +228,7 @@ impl<'a> Write for SmpBuffer<'a> {
#[derive(Copy, Clone, PartialEq)]
pub enum MsgType {
Echo,
+ ImageStateResponse,
ImageUploadResponse,
Unknown,
}
@@ -344,6 +347,9 @@ impl SmpReceiver {
(SMP_GROUP_OS, SMP_CMD_ID_ECHO) => {
self.msg_type = Some(MsgType::Echo);
}
+ (SMP_GROUP_IMAGE, SMP_CMD_ID_IMAGE_STATE) => {
+ self.msg_type = Some(MsgType::ImageStateResponse);
+ }
(SMP_GROUP_IMAGE, SMP_CMD_ID_IMAGE_UPLOAD) => {
self.msg_type = Some(MsgType::ImageUploadResponse);
}
Why this scored 35/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.