Various fixes

* Handle the `fram` error message in a way that mitigates a firmware bug

* CLI: resync on open

* upgrade base64

* update the firmware parsing regex

coldcard-cli/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "coldcard-cli"
-version = "0.12.0"
+version = "0.12.1"
 edition = "2021"
 authors = ["Alfred Hodler <[email protected]>"]
 license = "MIT"
@@ -17,8 +17,8 @@ name = "coldcard"
 path = "src/main.rs"
 
 [dependencies]
-coldcard = { version = "0.12.0", path = "../coldcard" }
-base64 = "0.13.0"
+coldcard = { version = "0.12.1", path = "../coldcard" }
+base64 = "0.21.7"
 clap = { version = "3.1.6", features = ["derive"] }
 hex = "0.4.3"
 hmac-sha256 = "1.1.7"

coldcard-cli/README.md

@@ -1,7 +1,6 @@
 # Coldcard CLI
 
-`coldcard-cli` is a CLI tool for interfacing with the [Coldcard](https://coldcard.com/) hardware wallet over USB.
-
+`coldcard-cli` is a firmware upgrade and general management tool for the [Coldcard](https://coldcard.com/) hardware wallet.
 
 Install it with:
 

coldcard-cli/src/fw_upgrade.rs

@@ -20,17 +20,18 @@ impl Release {
         let page = fetch_download_page()?;
 
         let mut found: Vec<_> = firmware_regex()
-            .find_iter(&page)
+            .captures_iter(&page)
             .map(|m| {
-                let version = version_regex().find(m.as_str()).unwrap();
+                let name = m.get(0).unwrap();
+                let version = m.get(1).unwrap();
                 let edge_marker = version.as_str().find('X');
                 let (version, is_edge) = match edge_marker {
                     Some(pos) => (Version::parse(&version.as_str()[1..pos]), true),
                     None => (Version::parse(&version.as_str()[1..]), false),
                 };
 
                 Release {
-                    name: m.as_str().to_owned(),
+                    name: name.as_str().to_owned(),
                     version: version.unwrap(),
                     is_edge,
                 }
@@ -94,10 +95,6 @@ fn fetch_download_page() -> Result<String, ureq::Error> {
 }
 
 fn firmware_regex() -> Regex {
-    Regex::new(r"[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]+-v[0-9]+\.[0-9]+\.[0-9]+X?(-mk.)?-coldcard.dfu")
+    Regex::new(r"[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]+-(v[0-9]+\.[0-9]+\.[0-9]+X?)(-mk.)?-coldcard.dfu")
         .unwrap()
 }
-
-fn version_regex() -> Regex {
-    Regex::new(r"v[0-9]+\.[0-9]+\.[0-9]+X?").unwrap()
-}

coldcard-cli/src/main.rs

@@ -5,7 +5,7 @@ use std::io::{Read, Seek, SeekFrom, Write};
 use std::path::PathBuf;
 
 use coldcard::protocol::{self, derivation_path, Response};
-use coldcard::{firmware, Backup, SignedMessage};
+use coldcard::{firmware, Backup, Options, SignedMessage};
 use coldcard::{util, XpubInfo};
 
 use clap::Parser;
@@ -286,8 +286,13 @@ fn handle(cli: Cli) -> Result<(), Error> {
     }
     .ok_or(Error::NoColdcardDetected)?;
 
-    let (mut cc, xpub_info) = api.open(sn, None)?;
-    cc.resync()?;
+    let (mut cc, xpub_info) = api.open(
+        sn,
+        Some(Options {
+            resync_on_open: true,
+            ..Default::default()
+        }),
+    )?;
 
     // check for MITM if requested
     let expected_xpub = cli.xpub;
@@ -453,7 +458,7 @@ fn handle(cli: Cli) -> Result<(), Error> {
                 }
             };
 
-            let encoded_sig = base64::encode(signature);
+            let encoded_sig = b64_encode(&signature);
             if armor {
                 let armor = format!(
                     "\
@@ -533,7 +538,7 @@ fn handle(cli: Cli) -> Result<(), Error> {
             let tx_string = match mode {
                 SignMode::Visualize => String::from_utf8(tx).unwrap(),
                 SignMode::VisualizeSigned => String::from_utf8(tx).unwrap(),
-                SignMode::Finalize if base64 => base64::encode(&tx),
+                SignMode::Finalize if base64 => b64_encode(&tx),
                 SignMode::Finalize => hex::encode(&tx),
             };
 
@@ -750,7 +755,7 @@ fn now() -> u64 {
 }
 
 fn calc_local_pincode(psbt_checksum: &[u8; 32], next_code: &str) -> Result<String, Error> {
-    let key = base64::decode(next_code).map_err(|_| Error::InvalidBase64)?;
+    let key = b64_decode(next_code).map_err(|_| Error::InvalidBase64)?;
     let digest = hmac_sha256::HMAC::mac(psbt_checksum, key);
     let last = digest[28..32].try_into().expect("cannot fail");
     let num = (u32::from_be_bytes(last) & 0x7FFFFFFF) % 1000000;
@@ -794,7 +799,7 @@ fn load_psbt(path: &PathBuf) -> Result<Vec<u8>, Error> {
         hex::decode(&data).map_err(|_| Error::InvalidPSBT)
     } else if &header[..6] == b"cHNidP" {
         trimmed(&mut data);
-        base64::decode(&data).map_err(|_| Error::InvalidPSBT)
+        b64_decode(&data).map_err(|_| Error::InvalidPSBT)
     } else if &header[..5] == b"psbt\xff" {
         Ok(data)
     } else {
@@ -934,3 +939,13 @@ impl ProgressBar {
         self.pb.tick();
     }
 }
+
+fn b64_encode(data: &[u8]) -> String {
+    use base64::Engine;
+    base64::engine::general_purpose::STANDARD.encode(data)
+}
+
+fn b64_decode(data: impl AsRef<[u8]>) -> Result<Vec<u8>, base64::DecodeError> {
+    use base64::Engine;
+    base64::engine::general_purpose::STANDARD.decode(data)
+}

coldcard/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "coldcard"
-version = "0.12.0"
+version = "0.12.1"
 edition = "2021"
 authors = ["Alfred Hodler <[email protected]>"]
 license = "MIT"
@@ -17,8 +17,8 @@ linux-static-libusb = ["hidapi/linux-static-libusb"]
 [dependencies]
 aes-ctr = "0.6.0"
 base58 = "0.2.0"
-bitcoin_hashes = "0.12.0"
+bitcoin_hashes = "0.13.0"
 hidapi = { version = "2.4.1", default-features = false }
-k256 = { version = "0.13.1", features = ["arithmetic"] }
-log = { version = "0.4.17", optional = true }
+k256 = { version = "0.13.3", features = ["arithmetic"] }
+log = { version = "0.4.20", optional = true }
 rand = "0.8.5"

coldcard/src/firmware.rs

@@ -53,8 +53,12 @@ impl Firmware {
             if (0..elements).next().is_some() {
                 let mut eprefix = [0_u8; 8];
                 stream.read_exact(&mut eprefix)?;
+                let address = decode_u32(eprefix.get(0..4))?;
                 let size = decode_u32(eprefix.get(4..8))?;
 
+                if address < 0x8008000 {
+                    return Err(Error::BadAddress);
+                }
                 if size % 256 != 0 {
                     return Err(Error::UnalignedSize);
                 }

coldcard/src/lib.rs

@@ -659,7 +659,7 @@ impl Coldcard {
         let mut rng = rand::thread_rng();
 
         for len in lengths {
-            let mut ping = Vec::new();
+            let mut ping = vec![0; len];
             rng.fill_bytes(&mut ping);
             let pong = self.send(Request::Ping(ping.clone()))?.into_binary()?;
             if ping != pong {
@@ -793,6 +793,8 @@ fn recv(
     read_buf: &mut [u8; 64],
 ) -> Result<Response, Error> {
     let mut data: Vec<u8> = Vec::new();
+    let mut packet_no = 0_u32;
+
     let (data, is_encrypted) = loop {
         #[cfg(feature = "log")]
         log::trace!("reading packet...");
@@ -803,11 +805,14 @@ fn recv(
         }
         let flag = read_buf[0];
         let is_last = flag & 0x80 != 0;
+        let is_fram = &read_buf[1..5] == b"fram" && packet_no == 0;
+        // firmware bug mitigation: `fram` responses are one packet but forget to set 0x80
+        let is_last = is_last || is_fram;
         let is_encrypted = flag & 0x40 != 0;
         let length = (flag & 0x3f) as usize;
 
         #[cfg(feature = "log")]
-        log::debug!("packet read ({} bytes)", length);
+        log::debug!("packet #{} read ({} bytes)", packet_no, length);
 
         // this is a small optimization to avoid vector allocation
         // when a response is sufficiently small to fit the buffer
@@ -819,6 +824,8 @@ fn recv(
                 break (&mut data, is_encrypted);
             }
         }
+
+        packet_no += 1;
     };
 
     if is_encrypted {
@@ -871,7 +878,7 @@ fn resync(cc: &mut hidapi::HidDevice, read_buf: &mut [u8; 64]) -> Result<(), Err
 
     read_junk(cc, read_buf)?;
 
-    let mut special_packet = vec![0xff_u8, 65];
+    let mut special_packet = [0xff_u8, 65];
     special_packet[0] = 0x00;
     special_packet[1] = 0x80;
     cc.write(&special_packet)?;