fix(core/delizia): correctly initialize `ConfirmAction` variants
What the developer wrote
Authored by Roman Zeyde
fix(core/delizia): correctly initialize `ConfirmAction` variants
[no changelog]
What changed, and why it matters
This commit fixes how the Trezor hardware wallet builds on-screen confirmation flows for the newer 'Delizia' interface. Previously, the code could register a confirmation page under the wrong internal state variant (for example, adding a confirmation screen to a flow that was not supposed to have one, or using the variant meant for flows with a menu when no menu existed). The patch makes the state variant explicit and passes it into the helper functions, ensuring each page is attached to the correct flow type. A mismatch could cause the device to show the wrong screen, ignore a button press, or behave unexpectedly when a user confirms or cancels an action.
Treat as a low-to-moderate reliability fix. Review whether the previous variant mismatch could lead to a skipped confirmation prompt or a stuck/crashed UI, and assess if any user action could be confirmed without the intended hold/tap interaction. If such a bypass is possible, request a security advisory and CVE; otherwise, include the fix in the next firmware release with a changelog note about UI flow robustness.
Security signals we found
UI state-machine inconsistency in security-critical confirmation flow
Possible wrong screen/page registration for hold-to-confirm / tap-to-confirm prompts
Possible mismatch between menu presence and confirmation state variant
Patch is marked [no changelog] and contains no explicit security disclosure
Evidence from the diff
In core/embed/rust/src/ui/layout_delizia/flow/confirm_action.rs, new_confirm_action_uni previously called create_menu and create_confirm, which internally decided which FlowController state variant to use based on extra and prompt_screen. The patch lifts that decision out of the helpers: it computes prompt_state (either ConfirmActionWithMenuAndConfirmation::Confirmation or ConfirmActionWithConfirmation::Confirmation) and menu_state (either ConfirmActionWithMenuAndConfirmation::Menu or ConfirmActionWithMenu::Menu) in the caller and passes them as explicit &'static dyn FlowController arguments. The helpers are also changed from conditionally doing nothing to always adding a page, and the caller only invokes them when needed. This prevents a page from being registered under an inconsistent state variant, which could corrupt the swipe-flow state machine.
Changed components
core/embed/rust/src/ui/layout_delizia/flow/confirm_action.rsTrezor Safe firmware UI flow for Delizia layoutConfirmAction swipe-flow state machineInspect captured patch +70 / −55
diff --git a/core/embed/rust/src/ui/layout_delizia/flow/confirm_action.rs b/core/embed/rust/src/ui/layout_delizia/flow/confirm_action.rs
index 4ca7e2277..e3ffc2334 100644
--- a/core/embed/rust/src/ui/layout_delizia/flow/confirm_action.rs
+++ b/core/embed/rust/src/ui/layout_delizia/flow/confirm_action.rs
@@ -317,9 +317,33 @@ fn new_confirm_action_uni<T: Component + Paginate + MaybeTrace + 'static>(
let mut flow = flow?;
flow.add_page(page, content)?;
- create_menu(&mut flow, &extra, prompt_screen)?;
- create_confirm(&mut flow, &extra, strings.subtitle, hold, prompt_screen)?;
+ let menu = match &extra {
+ ConfirmActionExtra::Menu(menu) => Some(menu),
+ _ => None,
+ };
+ let prompt_state: &'static dyn FlowController = match menu {
+ Some(_) => &ConfirmActionWithMenuAndConfirmation::Confirmation,
+ None => &ConfirmActionWithConfirmation::Confirmation,
+ };
+ let menu_state: &'static dyn FlowController = match prompt_screen {
+ Some(_) => &ConfirmActionWithMenuAndConfirmation::Menu,
+ None => &ConfirmActionWithMenu::Menu,
+ };
+
+ if let Some(menu_strings) = menu {
+ create_menu(&mut flow, menu_strings, menu_state)?;
+ }
+ if let Some(prompt_title) = prompt_screen {
+ create_confirm(
+ &mut flow,
+ &extra,
+ strings.subtitle,
+ hold,
+ prompt_title,
+ prompt_state,
+ )?;
+ }
Ok(flow)
}
@@ -357,36 +381,30 @@ fn create_flow(
fn create_menu(
flow: &mut SwipeFlow,
- extra: &ConfirmActionExtra,
- prompt_screen: Option<TString<'static>>,
+ menu_strings: &ConfirmActionMenuStrings,
+ menu_state: &'static dyn FlowController,
) -> Result<(), Error> {
- if let ConfirmActionExtra::Menu(menu_strings) = extra {
- let mut menu = VerticalMenu::empty();
- let mut menu_items = Vec::<usize, 2>::new();
+ let mut menu = VerticalMenu::empty();
+ let mut menu_items = Vec::<usize, 2>::new();
- if let Some(verb_info) = menu_strings.verb_info {
- menu = menu.item(theme::ICON_CHEVRON_RIGHT, verb_info);
- unwrap!(menu_items.push(MENU_ITEM_INFO));
- }
-
- menu = menu.danger(theme::ICON_CANCEL, menu_strings.verb_cancel);
- unwrap!(menu_items.push(MENU_ITEM_CANCEL));
+ if let Some(verb_info) = menu_strings.verb_info {
+ menu = menu.item(theme::ICON_CHEVRON_RIGHT, verb_info);
+ unwrap!(menu_items.push(MENU_ITEM_INFO));
+ }
- let content_menu = Frame::left_aligned("".into(), menu).with_cancel_button();
+ menu = menu.danger(theme::ICON_CANCEL, menu_strings.verb_cancel);
+ unwrap!(menu_items.push(MENU_ITEM_CANCEL));
- let content_menu = content_menu.map(move |msg| match msg {
- VerticalMenuChoiceMsg::Selected(i) => {
- let selected_item = menu_items[i];
- Some(FlowMsg::Choice(selected_item))
- }
- });
+ let content_menu = Frame::left_aligned("".into(), menu).with_cancel_button();
- if prompt_screen.is_some() {
- flow.add_page(&ConfirmActionWithMenuAndConfirmation::Menu, content_menu)?;
- } else {
- flow.add_page(&ConfirmActionWithMenu::Menu, content_menu)?;
+ let content_menu = content_menu.map(move |msg| match msg {
+ VerticalMenuChoiceMsg::Selected(i) => {
+ let selected_item = menu_items[i];
+ Some(FlowMsg::Choice(selected_item))
}
- }
+ });
+
+ flow.add_page(menu_state, content_menu)?;
Ok(())
}
@@ -396,40 +414,37 @@ fn create_confirm(
extra: &ConfirmActionExtra,
subtitle: Option<TString<'static>>,
hold: bool,
- prompt_screen: Option<TString<'static>>,
+ prompt_title: TString<'static>,
+ prompt_state: &'static dyn FlowController,
) -> Result<(), Error> {
- if let Some(prompt_title) = prompt_screen {
- let (prompt, prompt_action) = if hold {
- (
- PromptScreen::new_hold_to_confirm(),
- TR::instructions__hold_to_confirm.into(),
- )
- } else {
- (
- PromptScreen::new_tap_to_confirm(),
- TR::instructions__tap_to_confirm.into(),
- )
- };
-
- let mut content_confirm = Frame::left_aligned(prompt_title, SwipeContent::new(prompt))
- .with_footer(prompt_action, None)
- .with_swipe(Direction::Down, SwipeSettings::Default);
-
- if matches!(extra, ConfirmActionExtra::Menu(_)) {
- content_confirm = content_confirm.with_menu_button();
- }
+ let (prompt, prompt_action) = if hold {
+ (
+ PromptScreen::new_hold_to_confirm(),
+ TR::instructions__hold_to_confirm.into(),
+ )
+ } else {
+ (
+ PromptScreen::new_tap_to_confirm(),
+ TR::instructions__tap_to_confirm.into(),
+ )
+ };
- if let Some(subtitle) = subtitle {
- content_confirm = content_confirm.with_subtitle(subtitle);
- }
+ let mut content_confirm = Frame::left_aligned(prompt_title, SwipeContent::new(prompt))
+ .with_footer(prompt_action, None)
+ .with_swipe(Direction::Down, SwipeSettings::Default);
- let content_confirm = content_confirm.map(super::util::map_to_confirm);
+ if matches!(extra, ConfirmActionExtra::Menu(_)) {
+ content_confirm = content_confirm.with_menu_button();
+ }
- flow.add_page(
- &ConfirmActionWithMenuAndConfirmation::Confirmation,
- content_confirm,
- )?;
+ if let Some(subtitle) = subtitle {
+ content_confirm = content_confirm.with_subtitle(subtitle);
}
+
+ flow.add_page(
+ prompt_state,
+ content_confirm.map(super::util::map_to_confirm),
+ )?;
Ok(())
}
Why this scored 46/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.