feat(core/prodtest): allow alternative per-command CRC verification
What changed, and why it matters
This commit changes the Trezor production-test command-line interface so that CRC checksums can be applied either globally or per-command using a new 'checked-' prefix. It is a feature addition, not a fix for an exploitable vulnerability. The change does touch CRC parsing code, but the diff shows no obvious security bug such as buffer overflow or bypass. The main risk is that any mistake in the new CRC logic could weaken integrity checks for a low-privilege manufacturing/debug interface.
Review the new CRC computation boundaries carefully to ensure the 'checked-' prefix and final CRC token are excluded consistently, and that an attacker cannot truncate or mangle a command to bypass verification. Verify that the unused cstr_ends_with/cstr_remove_suffix helpers do not introduce dead code that could be linked into attack surface. No urgent action is indicated from the diff alone.
Security signals we found
Refactoring of authentication/integrity verification logic (CRC)
Addition of new command prefix that can enable per-command CRC enforcement
New string utility functions added but not visibly used in the changed code path
Evidence from the diff
The patch refactors CRC handling in core/embed/rtl/cli.c. It splits the previous global ‘crc_req’ flag into ‘crc_auto’ (persistent setting) and ‘crc_req’ (per-command flag), and introduces a ‘checked-’ command prefix that forces CRC verification for a single command even when CRC is otherwise disabled. It also adds cstr_ends_with() and cstr_remove_suffix() helpers in strutils.c/strutils.h, although those helpers do not appear to be used in the changed cli.c code. The CRC is now computed over the line after stripping the ‘checked-’ prefix and excluding the final 8-character hex CRC token. A minor operator-precedence fix is included in find_arg() (while condition parentheses).
Changed components
core/embed/rtl/cli.ccore/embed/rtl/inc/rtl/cli.hcore/embed/rtl/inc/rtl/strutils.hcore/embed/rtl/strutils.ccore/embed/projects/prodtest/README.mdInspect captured patch +102 / −38
diff --git a/core/embed/projects/prodtest/README.md b/core/embed/projects/prodtest/README.md
index 7abbc890..9973df4b 100644
--- a/core/embed/projects/prodtest/README.md
+++ b/core/embed/projects/prodtest/README.md
@@ -86,9 +86,9 @@ The CLI supports an optional CRC checksum for commands and responses to ensure d
When CRC is enabled, every command MUST include a CRC-32 checksum at the end of the line, preceded by a space.
Command Format:
-`<command> [<args>] <CRC32>`
+`<command> [<args>] <CRC32>` or `checked-<command> [<args> <CRC32>]`
-The checksum is calculated using the standard CRC-32 algorithm (polynomial `0xEDB88320`, initial value `0xFFFFFFFF`, and final XOR `0xFFFFFFFF`) over the command string excluding the ` <CRC32>` suffix.
+The checksum is calculated using the standard CRC-32 algorithm (polynomial `0xEDB88320`, initial value `0xFFFFFFFF`, and final XOR `0xFFFFFFFF`) over the command string excluding the checksum. In the `checked-<command> [<args> <CRC32>]` format, the `<CRC32>` is the checksum of the string starting after `checked-`.
The device also appends the checksum to every response line (including `OK`, `ERROR`, `PROGRESS`, and `#` traces).
@@ -97,8 +97,19 @@ Response Format:
Example with CRC enabled:
```
-ping ABC 70417631
-OK ABC E7193F16
+ping ABC 3240F7DC
+OK ABC 24DA4527
+```
+
+Example with `checked-` prefix (enforces CRC for a single command even if CRC is otherwise disabled):
+```
+checked-ping ABC 35F69145
+OK ABC 24DA4527
+```
+If the command has no arguments, the format is `checked-<command> <CRC32>`:
+```
+checked-ping D720F16C
+OK D38BF920
```
## List of commands
diff --git a/core/embed/rtl/cli.c b/core/embed/rtl/cli.c
index 1ba64762..dab7433d 100644
--- a/core/embed/rtl/cli.c
+++ b/core/embed/rtl/cli.c
@@ -11,6 +11,9 @@
#define ESC_COLOR_GRAY "\e[37m"
#define ESC_COLOR_RESET "\e[39m"
+#define CLI_CRC_PREFIX "checked-"
+#define CLI_CRC_LENGTH 8
+
#define CRC32_INITIAL 0xFFFFFFFF
#define CRC32_POLYNOMIAL 0xEDB88320
@@ -229,9 +232,9 @@ void cli_abort(cli_t* cli) { cli->aborted = true; }
bool cli_aborted(cli_t* cli) { return cli->aborted; }
-void cli_enable_crc(cli_t* cli) { cli->crc_req = true; }
+void cli_enable_crc(cli_t* cli) { cli->crc_auto = true; }
-void cli_disable_crc(cli_t* cli) { cli->crc_req = false; }
+void cli_disable_crc(cli_t* cli) { cli->crc_auto = false; }
// Finds a command record by name
//
@@ -541,6 +544,7 @@ static void cli_clear_line(cli_t* cli) {
cli->hist_idx = 0;
cli->hist_prefix = 0;
cli->response_crc = CRC32_INITIAL;
+ cli->crc_req = cli->crc_auto;
memset(cli->line_buffer, 0, sizeof(cli->line_buffer));
}
@@ -570,6 +574,12 @@ static bool cli_split_args(cli_t* cli) {
cli->cmd_name = cstr_token(&buf);
cli->args_count = 0;
+ // Single crc check?
+ if (cstr_starts_with(cli->line_buffer, CLI_CRC_PREFIX)) {
+ cli->cmd_name += strlen(CLI_CRC_PREFIX);
+ cli->crc_req = true;
+ }
+
while (*buf != '\0' && cli->args_count < CLI_MAX_ARGS) {
const char* arg = cstr_token(&buf);
if (*arg != '\0') {
@@ -623,39 +633,17 @@ const cli_command_t* cli_process_io(cli_t* cli) {
goto cleanup;
}
- // Handle optional CRC check
- if (cli->crc_req) {
- size_t len = strlen(cli->line_buffer);
- if (len >= 9) {
- char* space = &cli->line_buffer[len - 9];
- if (*space == ' ') {
- uint32_t received_crc;
- if (cstr_parse_uint32(space + 1, 16, &received_crc)) {
- uint32_t calculated_crc =
- ~cli_crc32(CRC32_INITIAL, cli->line_buffer, len - 9);
- if (calculated_crc == received_crc) {
- *space = '\0';
- } else {
- cli_error(cli, CLI_ERROR_INVALID_CRC, "Expected %08X, got %08X",
- calculated_crc, received_crc);
- goto cleanup;
- }
- } else {
- cli_error(cli, CLI_ERROR_INVALID_CRC, "Invalid CRC format");
- goto cleanup;
- }
- } else {
- cli_error(cli, CLI_ERROR_INVALID_CRC, "CRC suffix missing");
- goto cleanup;
- }
- } else {
- cli_error(cli, CLI_ERROR_INVALID_CRC, "Line too short for CRC");
- goto cleanup;
- }
- }
-
cli_history_add(cli, cli->line_buffer);
+ // Calculate CRC of the command line (excluding the expected CRC suffix)
+ // (we may not use the value if crc is not requested)
+ size_t crc_offset = cstr_starts_with(cli->line_buffer, CLI_CRC_PREFIX)
+ ? strlen(CLI_CRC_PREFIX)
+ : 0;
+ uint32_t calculated_crc =
+ ~cli_crc32(CRC32_INITIAL, cli->line_buffer + crc_offset,
+ MAX((int)cli->line_len - (int)crc_offset - CLI_CRC_LENGTH, 0));
+
// Split command line into arguments
if (!cli_split_args(cli)) {
cli_error(cli, CLI_ERROR_FATAL, "Too many arguments.");
@@ -686,6 +674,34 @@ const cli_command_t* cli_process_io(cli_t* cli) {
goto cleanup;
}
+ if (cli->crc_req) {
+ if (cli->args_count < 1) {
+ cli->crc_req = false;
+ cli_error(cli, CLI_ERROR_INVALID_CRC, "CRC suffix missing");
+ goto cleanup;
+ }
+
+ uint32_t crc = 0;
+
+ const char* crc_str = cli_nth_arg(cli, cli->args_count - 1);
+
+ if (strlen(crc_str) != CLI_CRC_LENGTH ||
+ !cstr_parse_uint32(crc_str, 16, &crc)) {
+ cli->crc_req = false;
+ cli_error(cli, CLI_ERROR_INVALID_CRC, "Invalid CRC format");
+ goto cleanup;
+ }
+
+ if (calculated_crc != crc) {
+ cli->crc_req = false;
+ cli_error(cli, CLI_ERROR_INVALID_CRC, "Expected %08X, got %08X",
+ calculated_crc, crc);
+ goto cleanup;
+ }
+
+ --cli->args_count;
+ }
+
// Find the command handler
const cli_command_t* found_cmd = cli_find_command(cli, cli->cmd_name);
@@ -728,7 +744,7 @@ static int find_arg(const cli_command_t* cmd, const char* name) {
// Extract argument name
const char* s = p;
- while (*p != '\0' && (*p != '>' && *p != ']')) {
+ while (*p != '\0' && *p != '>' && *p != ']') {
p++;
}
diff --git a/core/embed/rtl/inc/rtl/cli.h b/core/embed/rtl/inc/rtl/cli.h
index 87bc9666..65c6b23f 100644
--- a/core/embed/rtl/inc/rtl/cli.h
+++ b/core/embed/rtl/inc/rtl/cli.h
@@ -145,6 +145,8 @@ struct cli {
uint32_t response_crc;
/** CRC was requested for the current command */
bool crc_req;
+ /** CRC is enabled for all subsequent commands */
+ bool crc_auto;
};
/** Initializes the command line structure */
diff --git a/core/embed/rtl/inc/rtl/strutils.h b/core/embed/rtl/inc/rtl/strutils.h
index ed4fae3f..05b6a770 100644
--- a/core/embed/rtl/inc/rtl/strutils.h
+++ b/core/embed/rtl/inc/rtl/strutils.h
@@ -67,6 +67,24 @@ const char* cstr_skip_whitespace(const char* str);
*/
bool cstr_starts_with(const char* str, const char* prefix);
+/**
+ * Returns true if the null-terminated C-string ends with the suffix.
+ *
+ * @param str The null-terminated C-string to check
+ * @param suffix The suffix to look for
+ * @return true if the string ends with the suffix, false otherwise
+ */
+bool cstr_ends_with(const char* str, const char* suffix);
+
+/**
+ * Removes the suffix from the null-terminated C-string if it exists.
+ *
+ * @param str The null-terminated C-string to modify
+ * @param suffix The suffix to remove
+ * @return true if the suffix was found and removed, false otherwise
+ */
+bool cstr_remove_suffix(char* str, const char* suffix);
+
/**
* Decodes the string as a hexadecimal string and writes the binary data to the
* destination buffer.
diff --git a/core/embed/rtl/strutils.c b/core/embed/rtl/strutils.c
index 4036c7e3..15c9e798 100644
--- a/core/embed/rtl/strutils.c
+++ b/core/embed/rtl/strutils.c
@@ -58,6 +58,23 @@ bool cstr_starts_with(const char* str, const char* prefix) {
return strlen(str) >= prefix_len && 0 == strncmp(str, prefix, prefix_len);
}
+bool cstr_ends_with(const char* str, const char* suffix) {
+ size_t str_len = strlen(str);
+ size_t suffix_len = strlen(suffix);
+ if (str_len < suffix_len) {
+ return false;
+ }
+ return 0 == strcmp(str + str_len - suffix_len, suffix);
+}
+
+bool cstr_remove_suffix(char* str, const char* suffix) {
+ if (cstr_ends_with(str, suffix)) {
+ str[strlen(str) - strlen(suffix)] = '\0';
+ return true;
+ }
+ return false;
+}
+
static inline bool parse_nibble(char c, uint32_t* value) {
uint8_t nibble = 0;
Why this scored 19/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.