Skip to content

Commit

Permalink
Merge pull request #35 from LedgerHQ/y333/stax_workaround
Browse files Browse the repository at this point in the history 
Fix Stax/Flex: incomplete APDU is returned
  • Loading branch information
@yogh333
yogh333 authored Oct 9, 2024
2 parents bacad90 + de6f214 commit d64cf9f
Show file tree
Hide file tree
Showing 9 changed files with 46 additions and 57 deletions.
6 changes: 3 additions & 3 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions starknet/Cargo.toml
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
[package]
name = "starknet"
version = "2.1.0"
version = "2.1.1"
edition = "2021"
authors = ["Ledger"]

[dependencies]
ledger_device_sdk = "1.17.2"
ledger_device_sdk = "1.17.3"
ledger_secure_sdk_sys = { version = "1.5.0", features = ["heap"]}
include_gif = "1.2.0"
hex = { version = "0.4", default-features = false, features = ["alloc"]}
Expand Down
6 changes: 0 additions & 6 deletions starknet/src/crypto.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,7 @@ impl From<CryptoError> for Reply {

/// Helper function that signs with ECDSA in deterministic nonce
pub fn sign_hash(ctx: &mut Ctx) -> Result<(), CryptoError> {
// ledger_device_sdk::testing::debug_print("Before shift: ");
// ledger_device_sdk::testing::debug_print(&ctx.hash.m_hash.to_hex_string());
// ledger_device_sdk::testing::debug_print("\n");
poseidon::poseidon_shift(&mut ctx.hash.m_hash);
// ledger_device_sdk::testing::debug_print("After shift: ");
// ledger_device_sdk::testing::debug_print(&ctx.hash.m_hash.to_hex_string());
// ledger_device_sdk::testing::debug_print("\n");

match Stark256::derive_from_path(ctx.bip32_path.as_ref())
.deterministic_sign(ctx.hash.m_hash.value.as_ref())
Expand Down
18 changes: 2 additions & 16 deletions starknet/src/display.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,8 @@ use crate::{
erc20::{ERC20_TOKENS, TRANSFER},
types::FieldElement,
};
use alloc::format;
use include_gif::include_gif;
use ledger_device_sdk::{io::Comm, testing};
use ledger_device_sdk::io::Comm;

use crate::context::{Ctx, Transaction};

Expand All @@ -26,7 +25,6 @@ use ledger_device_sdk::nbgl::{
pub fn show_tx(ctx: &mut Ctx) -> Option<bool> {
match support_clear_sign(&ctx.tx) {
Some(t) => {
testing::debug_print("Clear sign supported !!! \n");
let tx = &ctx.tx;
let call = &tx.calls[0];

Expand All @@ -35,8 +33,6 @@ pub fn show_tx(ctx: &mut Ctx) -> Option<bool> {
let to = call.calldata[0].to_hex_string();
let amount = call.calldata[1].to_dec_string(Some(ERC20_TOKENS[t].decimals));

testing::debug_print("Compute fees \n");

let max_fees_str = match tx.version {
FieldElement::ONE => {
let mut max_fees_str = tx.max_fee.to_dec_string(None);
Expand All @@ -58,8 +54,6 @@ pub fn show_tx(ctx: &mut Ctx) -> Option<bool> {
}
};

testing::debug_print("Compute fees OK \n");

let my_fields = [
Field {
name: "From",
Expand All @@ -83,11 +77,6 @@ pub fn show_tx(ctx: &mut Ctx) -> Option<bool> {
},
];

testing::debug_print(&format!(
"Token: {}\nTo: {}\nAmount: {}\n",
token, to, amount
));

#[cfg(not(any(target_os = "stax", target_os = "flex")))]
{
let my_review = MultiFieldReview::new(
Expand Down Expand Up @@ -115,10 +104,7 @@ pub fn show_tx(ctx: &mut Ctx) -> Option<bool> {
Some(review.show(&my_fields))
}
}
None => {
testing::debug_print("Clear sign not supported !!! \n");
None
}
None => None,
}
}

Expand Down
69 changes: 39 additions & 30 deletions starknet/src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ extern "C" fn sample_main() {
// or an APDU command
if let io::Event::Command(ins) = display::main_ui(&mut comm) {
match handle_apdu(&mut comm, &ins, &mut ctx) {
Ok(()) => comm.reply_ok(),
Ok(data) => {
comm.append(data.as_slice());
comm.reply_ok()
}
Err(sw) => comm.reply(sw),
}
}
Expand All @@ -54,7 +57,10 @@ extern "C" fn sample_main() {
// Wait for an APDU command
let ins: Ins = comm.next_command();
match handle_apdu(&mut comm, &ins, &mut ctx) {
Ok(()) => comm.reply_ok(),
Ok(data) => {
comm.append(data.as_slice());
comm.reply_ok()
}
Err(sw) => comm.reply(sw),
}
}
Expand Down Expand Up @@ -101,19 +107,22 @@ use ledger_device_sdk::io::Reply;

const SIG_LENGTH: u8 = 0x41;

fn handle_apdu(comm: &mut io::Comm, ins: &Ins, ctx: &mut Ctx) -> Result<(), Reply> {
fn handle_apdu(comm: &mut io::Comm, ins: &Ins, ctx: &mut Ctx) -> Result<Vec<u8>, Reply> {
if comm.rx == 0 {
return Err(io::StatusWords::NothingReceived.into());
}

let apdu_header = comm.get_apdu_metadata();

let mut rdata: Vec<u8> = Vec::new();

match ins {
Ins::GetVersion => {
let version_major = env!("CARGO_PKG_VERSION_MAJOR").parse::<u8>().unwrap();
let version_minor = env!("CARGO_PKG_VERSION_MINOR").parse::<u8>().unwrap();
let version_patch = env!("CARGO_PKG_VERSION_PATCH").parse::<u8>().unwrap();
comm.append([version_major, version_minor, version_patch].as_slice());

rdata.extend_from_slice([version_major, version_minor, version_patch].as_slice());
}
Ins::GetPubkey { display } => {
ctx.reset();
Expand All @@ -138,7 +147,7 @@ fn handle_apdu(comm: &mut io::Comm, ins: &Ins, ctx: &mut Ctx) -> Result<(), Repl
true => display::pkey_ui(key.as_ref(), ctx),
};
if ret {
comm.append(key.as_ref());
rdata.extend_from_slice(key.as_ref());
} else {
return Err(io::StatusWords::UserCancelled.into());
}
Expand All @@ -164,10 +173,10 @@ fn handle_apdu(comm: &mut io::Comm, ins: &Ins, ctx: &mut Ctx) -> Result<(), Repl
match display::show_hash(ctx, false) {
true => {
crypto::sign_hash(ctx).unwrap();
comm.append([0x41].as_slice());
comm.append(ctx.hash.r.as_ref());
comm.append(ctx.hash.s.as_ref());
comm.append([ctx.hash.v].as_slice());
rdata.extend_from_slice([0x41].as_slice());
rdata.extend_from_slice(ctx.hash.r.as_ref());
rdata.extend_from_slice(ctx.hash.s.as_ref());
rdata.extend_from_slice([ctx.hash.v].as_slice());
}
false => {
return Err(io::StatusWords::UserCancelled.into());
Expand Down Expand Up @@ -213,13 +222,13 @@ fn handle_apdu(comm: &mut io::Comm, ins: &Ins, ctx: &mut Ctx) -> Result<(), Repl
true => {
display::show_pending();
ctx.hash.m_hash = crypto::tx_hash(&ctx.tx);
comm.append(ctx.hash.m_hash.value.as_ref());
rdata.extend_from_slice(ctx.hash.m_hash.value.as_ref());
crypto::sign_hash(ctx).unwrap();
rdata.extend_from_slice([SIG_LENGTH].as_slice());
rdata.extend_from_slice(ctx.hash.r.as_ref());
rdata.extend_from_slice(ctx.hash.s.as_ref());
rdata.extend_from_slice([ctx.hash.v].as_slice());
display::show_status(true, ctx);
comm.append([SIG_LENGTH].as_slice());
comm.append(ctx.hash.r.as_ref());
comm.append(ctx.hash.s.as_ref());
comm.append([ctx.hash.v].as_slice());
}
false => {
display::show_status(false, ctx);
Expand All @@ -231,13 +240,13 @@ fn handle_apdu(comm: &mut io::Comm, ins: &Ins, ctx: &mut Ctx) -> Result<(), Repl
ctx.hash.m_hash = crypto::tx_hash(&ctx.tx);
match display::show_hash(ctx, true) {
true => {
comm.append(ctx.hash.m_hash.value.as_ref());
rdata.extend_from_slice(ctx.hash.m_hash.value.as_ref());
crypto::sign_hash(ctx).unwrap();
rdata.extend_from_slice([SIG_LENGTH].as_slice());
rdata.extend_from_slice(ctx.hash.r.as_ref());
rdata.extend_from_slice(ctx.hash.s.as_ref());
rdata.extend_from_slice([ctx.hash.v].as_slice());
display::show_status(true, ctx);
comm.append([SIG_LENGTH].as_slice());
comm.append(ctx.hash.r.as_ref());
comm.append(ctx.hash.s.as_ref());
comm.append([ctx.hash.v].as_slice());
}
false => {
display::show_status(false, ctx);
Expand Down Expand Up @@ -284,13 +293,13 @@ fn handle_apdu(comm: &mut io::Comm, ins: &Ins, ctx: &mut Ctx) -> Result<(), Repl
true => {
display::show_pending();
ctx.hash.m_hash = crypto::tx_hash(&ctx.tx);
comm.append(ctx.hash.m_hash.value.as_ref());
rdata.extend_from_slice(ctx.hash.m_hash.value.as_ref());
crypto::sign_hash(ctx).unwrap();
rdata.extend_from_slice([0x41].as_slice());
rdata.extend_from_slice(ctx.hash.r.as_ref());
rdata.extend_from_slice(ctx.hash.s.as_ref());
rdata.extend_from_slice([ctx.hash.v].as_slice());
display::show_status(true, ctx);
comm.append([0x41].as_slice());
comm.append(ctx.hash.r.as_ref());
comm.append(ctx.hash.s.as_ref());
comm.append([ctx.hash.v].as_slice());
}
false => {
display::show_status(false, ctx);
Expand All @@ -302,13 +311,13 @@ fn handle_apdu(comm: &mut io::Comm, ins: &Ins, ctx: &mut Ctx) -> Result<(), Repl
ctx.hash.m_hash = crypto::tx_hash(&ctx.tx);
match display::show_hash(ctx, true) {
true => {
comm.append(ctx.hash.m_hash.value.as_ref());
rdata.extend_from_slice(ctx.hash.m_hash.value.as_ref());
crypto::sign_hash(ctx).unwrap();
rdata.extend_from_slice([0x41].as_slice());
rdata.extend_from_slice(ctx.hash.r.as_ref());
rdata.extend_from_slice(ctx.hash.s.as_ref());
rdata.extend_from_slice([ctx.hash.v].as_slice());
display::show_status(true, ctx);
comm.append([0x41].as_slice());
comm.append(ctx.hash.r.as_ref());
comm.append(ctx.hash.s.as_ref());
comm.append([ctx.hash.v].as_slice());
}
false => {
display::show_status(false, ctx);
Expand Down Expand Up @@ -375,5 +384,5 @@ fn handle_apdu(comm: &mut io::Comm, ins: &Ins, ctx: &mut Ctx) -> Result<(), Repl
}
}
}
Ok(())
Ok(rdata)
}
Binary file modified tests/snapshots/flex/test_app_mainmenu/00001.png
View file
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/snapshots/nanosp/test_app_mainmenu/00001.png
View file
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/snapshots/nanox/test_app_mainmenu/00001.png
View file
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/snapshots/stax/test_app_mainmenu/00001.png
View file
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

0 comments on commit d64cf9f

Please sign in to comment.