Skip to content

Initialize and check the stack canary at every io_tx/io_rx call #333

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
bigspider wants to merge 1 commit into from
Closed

Initialize and check the stack canary at every io_tx/io_rx call #333

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions ledger_secure_sdk_sys/src/seph.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,47 @@
use crate::{os_io_rx_evt, os_io_tx_cmd};

mod canary {
// This module provides stack overflow protection by initializing and checking
// a canary value to detect if the stack has grown too much and is overlapping
// with the .bss section. The canary is checked on every APDU I/O operation and
// will panic if corruption is detected.
// This might later be removed if such protection is provided in the C SDK.

extern "C" {
/// Stack canary symbol provided by the linker script
static mut app_stack_canary: u32;
}

const APP_STACK_CANARY_MAGIC: u32 = 0xDEAD0031;
static mut CANARY_INITIALIZED: bool = false;
Comment thread
iartemov-ledger marked this conversation as resolved.

/// Initialize the stack canary with the magic value
fn init_canary() {
unsafe {
core::ptr::write_volatile(&raw mut app_stack_canary, APP_STACK_CANARY_MAGIC);
CANARY_INITIALIZED = true;
}
}

/// Ensure canary is initialized and check if it's still intact
#[inline]
pub(super) fn init_and_check() {
unsafe {
if !CANARY_INITIALIZED {
init_canary();
Copy link
Copy Markdown
Contributor

@iartemov-ledger iartemov-ledger Feb 5, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The laziness has pros (no explicit init is needed), but also cons (if the stack has been already corrupted the first lazy init will re-patch incorrect marker value).

Copy link
Copy Markdown
Contributor Author

@bigspider bigspider Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair point, but I would prefer to avoid for the app to have a specific 'canary initialization' call - especially since we might later drop this feature once the C SDK implements it on its own.

If you have any idea of a clean way to make sure the canary is initialized at loading time, that would be a better solution.

Copy link
Copy Markdown
Contributor

@iartemov-ledger iartemov-ledger Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not know Rust C as well. @yogh333 , may you propose anything?

}

let canary_value = core::ptr::read_volatile(&raw const app_stack_canary);
if canary_value != APP_STACK_CANARY_MAGIC {
panic!("Stack canary corruption detected!");
}
}
}
}

/// Receive the next APDU into 'buffer'
pub fn io_rx(buffer: &mut [u8], check_se_event: bool) -> i32 {
canary::init_and_check();
unsafe {
os_io_rx_evt(
buffer.as_ptr() as _,
Expand All @@ -13,6 +53,7 @@ pub fn io_rx(buffer: &mut [u8], check_se_event: bool) -> i32 {
}

pub fn io_tx(apdu_type: u8, buffer: &[u8], length: usize) -> i32 {
canary::init_and_check();
unsafe {
os_io_tx_cmd(
apdu_type,
Expand Down
Loading