Merge pull request #126 from Zondax/nat/fix_msg_prefix

Use varuint to prefix message len

app/Makefile.version

@@ -1,3 +1,3 @@
 APPVERSION_M=0
-APPVERSION_N=22
-APPVERSION_P=9
+APPVERSION_N=23
+APPVERSION_P=0

app/rust/Cargo.toml

@@ -15,7 +15,6 @@ crate-type = ["staticlib"]
 no-std-compat = { version = "0.4.1" }
 numtoa = "0.2.4"
 arrayref = "0.3.6"
-lexical-core = { version = "0.8", features = ["parse-integers"], default-features = false }
 base64 = {version = "0.13.0", default-features = false }
 hex = { version = "0.4", default-features = false }
 

app/rust/src/parser/message.rs

@@ -1,7 +1,8 @@
 #![allow(clippy::missing_safety_doc)]
-use super::error::ParserError;
+use super::{error::ParserError, read_varint};
 use crate::zxformat::{pageString, Writer};
 use core::fmt::Write;
+use nom::bytes::complete::take;
 
 // The lenght of \x17Stacks Signed Message:
 const BYTE_STRING_HEADER_LEN: usize = "\x17Stacks Signed Message:\n".as_bytes().len();
@@ -52,11 +53,7 @@ impl<'a> Message<'a> {
 
 #[repr(C)]
 #[derive(Debug, Copy, Clone, PartialEq)]
-pub struct ByteString<'a> {
-    data: &'a [u8],
-    at: usize,
-    len: usize,
-}
+pub struct ByteString<'a>(&'a [u8]);
 
 impl<'a> ByteString<'a> {
     pub fn maybe_byte_string(data: &'a [u8]) -> bool {
@@ -69,43 +66,26 @@ impl<'a> ByteString<'a> {
         data.len() > BYTE_STRING_HEADER_LEN && &data[..BYTE_STRING_HEADER_LEN] == msg_bytes
     }
 
-    fn msg_len(data: &'a [u8]) -> Result<(usize, usize), ParserError> {
+    // returns the message content
+    fn get_msg(data: &'a [u8]) -> Result<&'a [u8], ParserError> {
         if data.is_empty() || !data.is_ascii() {
             return Err(ParserError::parser_invalid_bytestr_message);
         }
-        let mut digit_count = 0usize;
-        let mut len;
-        for (index, c) in data.iter().enumerate() {
-            if (*c as char).is_ascii_digit() {
-                digit_count += 1;
-
-                len = lexical_core::parse::<usize>(&data[..=index])
-                    .map_err(|_| ParserError::parser_value_out_of_range)?;
-
-                let data_len = data.len() - digit_count;
-
-                // no trailing zeros
-                if len == 0 && data_len > 1 {
-                    return Err(ParserError::parser_invalid_bytestr_message);
-                }
-
-                if len == data_len {
-                    return Ok((digit_count, len));
-                }
-            } else {
-                break;
-            }
-        }
-        Err(ParserError::parser_invalid_bytestr_message)
+
+        let (rem, len) =
+            read_varint(data).map_err(|_| ParserError::parser_invalid_bytestr_message)?;
+        let (_, message_content) = take::<_, _, ParserError>(len as usize)(rem)
+            .map_err(|_| ParserError::parser_invalid_bytestr_message)?;
+
+        Ok(message_content)
     }
 
     pub fn from_bytes(data: &'a [u8]) -> Result<Self, ParserError> {
         if !Self::contain_header(data) {
             return Err(ParserError::parser_invalid_bytestr_message);
         }
-        let (at, len) = Self::msg_len(&data[BYTE_STRING_HEADER_LEN..])?;
-        let at = BYTE_STRING_HEADER_LEN + at;
-        Ok(Self { data, at, len })
+        let message = Self::get_msg(&data[BYTE_STRING_HEADER_LEN..])?;
+        Ok(Self(message))
     }
 
     pub const fn num_items(&self) -> u8 {
@@ -127,8 +107,7 @@ impl<'a> ByteString<'a> {
                 .write_str("Sign Message")
                 .map_err(|_| ParserError::parser_unexpected_buffer_end)?;
 
-            let len = self.at + self.len;
-            pageString(out_value, &self.data[self.at..len], page_idx)
+            pageString(out_value, self.0, page_idx)
         } else {
             Err(ParserError::parser_display_idx_out_of_range)
         }
@@ -141,53 +120,58 @@ mod test {
 
     use super::*;
 
-    fn built_message(len: usize, data: &str) -> String {
-        format!("\x17Stacks Signed Message:\n{}{}", len, data)
+    fn built_message(len: usize, data: &str) -> Vec<u8> {
+        let header = "\x17Stacks Signed Message:\n".as_bytes();
+        let mut vec = vec![];
+        vec.extend_from_slice(header);
+        vec.push(len as u8);
+        vec.extend_from_slice(data.as_bytes());
+        vec
     }
 
     #[test]
     fn test_non_ascii_byte_string() {
         let no_ascii = "Test-love: ❤️";
         let no_ascii = built_message(no_ascii.len(), no_ascii);
-        let msg = ByteString::from_bytes(no_ascii.as_bytes());
+        let msg = ByteString::from_bytes(&no_ascii);
         assert!(msg.is_err());
     }
 
     #[test]
     fn test_valid_byte_string() {
         let data = "byte_string_valid";
         let m = built_message(data.len(), data);
-        let msg = ByteString::from_bytes(m.as_bytes());
+        let msg = ByteString::from_bytes(&m);
         assert!(msg.is_ok());
         let msg = msg.unwrap();
-        assert_eq!(&msg.data[msg.at..], data.as_bytes());
+        assert_eq!(msg.0, data.as_bytes());
     }
 
     #[test]
     fn test_valid_starts_with_number() {
         let data = "1_byte_string_valid";
         let m = built_message(data.len(), data);
-        let msg = ByteString::from_bytes(m.as_bytes());
+        let msg = ByteString::from_bytes(&m);
         assert!(msg.is_ok());
         let msg = msg.unwrap();
-        assert_eq!(&msg.data[msg.at..], data.as_bytes());
+        assert_eq!(msg.0, data.as_bytes());
     }
 
     #[test]
     fn test_empty_byte_string() {
         let data = "";
         let m = built_message(data.len(), data);
-        let msg = ByteString::from_bytes(m.as_bytes());
+        let msg = ByteString::from_bytes(&m);
         assert!(msg.is_ok());
         let msg = msg.unwrap();
-        assert_eq!(&msg.data[msg.at..], data.as_bytes());
+        assert_eq!(msg.0, data.as_bytes());
     }
 
     #[test]
     fn test_wrong_len_byte_string() {
         let m = "byte_string_valid";
         let m = built_message(34, m);
-        let msg = ByteString::from_bytes(m.as_bytes());
+        let msg = ByteString::from_bytes(&m);
         assert!(msg.is_err());
     }
 

app/rust/src/parser/mod.rs

@@ -10,6 +10,7 @@ mod spending_condition;
 mod transaction;
 mod transaction_auth;
 mod transaction_payload;
+mod utils;
 mod value;
 pub use error::ParserError;
 pub use ffi::{_getItem, _getNumItems, _parser_init, _read, fp_uint64_to_str};
@@ -20,4 +21,5 @@ pub use parser_common::{SignerId, TransactionVersion};
 pub use post_condition::{FungibleConditionCode, TransactionPostCondition};
 pub use transaction::Transaction;
 pub use transaction_auth::TransactionAuth;
+pub use utils::*;
 pub use value::{Value, ValueId};

app/rust/src/parser/parsed_obj.rs

@@ -271,9 +271,9 @@ mod test {
 
     #[test]
     fn read_message() {
-        let msg = "Hello World";
-        let data = format!("\x17Stacks Signed Message:\n{}{}", msg.len(), msg);
-        let mut parsed_obj = ParsedObj::from_bytes(data.as_bytes()).expect("Invalid input data");
+        let blob = "17537461636b73205369676e6564204d6573736167653a0a0b48656c6c6f20576f726c64";
+        let blob = hex::decode(blob).unwrap();
+        let mut parsed_obj = ParsedObj::from_bytes(&blob).expect("Invalid input data");
         ParsedObj::validate(&mut parsed_obj).unwrap();
     }
 

app/rust/src/parser/parser_common.rs

@@ -1,11 +1,8 @@
 #![allow(non_camel_case_types, non_upper_case_globals, non_snake_case)]
 #![allow(clippy::upper_case_acronyms)]
 use nom::{
-    branch::permutation,
     bytes::complete::take,
-    combinator::{iterator, map_parser},
-    error::ErrorKind,
-    number::complete::{be_u32, le_u32, le_u64, le_u8},
+    number::complete::{be_u32, le_u8},
 };
 
 use crate::parser::{c32, error::ParserError};

app/rust/src/parser/post_condition.rs

@@ -1,19 +1,16 @@
 use arrayvec::ArrayVec;
-use core::fmt::{self, Write};
+use core::fmt::Write;
 use nom::{
     bytes::complete::take,
-    error::ErrorKind,
     number::complete::{be_u64, le_u8},
 };
 
 use super::error::ParserError;
 
 use super::parser_common::{
-    u8_with_limits, AssetInfo, AssetInfoId, ClarityName, ContractName, StacksAddress,
-    C32_ENCODED_ADDRS_LENGTH, HASH160_LEN, MAX_STRING_LEN, NUM_SUPPORTED_POST_CONDITIONS,
-    STX_DECIMALS,
+    AssetInfo, ContractName, StacksAddress, C32_ENCODED_ADDRS_LENGTH, HASH160_LEN, STX_DECIMALS,
 };
-use crate::parser::{c32, fp_uint64_to_str, value::Value};
+use crate::parser::value::Value;
 use crate::zxformat;
 
 #[repr(u8)]

app/rust/src/parser/spending_condition.rs

@@ -1,17 +1,16 @@
 use nom::{
     bytes::complete::take,
-    error::ErrorKind,
-    number::complete::{be_u16, be_u32, be_u64, le_u8},
+    number::complete::{be_u16, be_u32, be_u64},
 };
 
 use arrayvec::ArrayVec;
 
+use crate::parser::c32;
 use crate::parser::error::ParserError;
 use crate::parser::parser_common::{
-    HashMode, SignerId, TransactionVersion, C32_ENCODED_ADDRS_LENGTH, HASH160_LEN, SIGNATURE_LEN,
+    HashMode, TransactionVersion, C32_ENCODED_ADDRS_LENGTH, SIGNATURE_LEN,
 };
-use crate::parser::{c32, ffi::fp_uint64_to_str};
-use crate::{bolos::c_zemu_log_stack, check_canary, zxformat};
+use crate::{check_canary, zxformat};
 
 // this includes:
 // 16-byte origin fee and nonce

app/rust/src/parser/transaction.rs

@@ -1,4 +1,4 @@
-use core::fmt::{self, Write};
+use core::fmt::Write;
 use nom::{
     branch::permutation,
     bytes::complete::take,

app/rust/src/parser/transaction_auth.rs

@@ -1,10 +1,5 @@
-use nom::{
-    bytes::complete::take,
-    error::ErrorKind,
-    number::complete::{be_u64, le_u8},
-};
+use nom::number::complete::le_u8;
 
-use crate::bolos::c_zemu_log_stack;
 use crate::check_canary;
 use crate::parser::{
     error::ParserError,

app/rust/src/parser/transaction_payload.rs

@@ -1,9 +1,6 @@
-use core::fmt::{self, Write};
+use core::fmt::Write;
 use nom::{
-    branch::permutation,
     bytes::complete::take,
-    combinator::{iterator, map_parser},
-    error::ErrorKind,
     number::complete::{be_i128, be_u128, be_u32, be_u64, le_u8},
     sequence::tuple,
 };
@@ -13,15 +10,12 @@ use numtoa::NumToA;
 
 use crate::parser::error::ParserError;
 use crate::parser::parser_common::{
-    u8_with_limits, AssetInfo, AssetInfoId, ClarityName, ContractName, PrincipalData,
-    StacksAddress, StacksString, StandardPrincipal, C32_ENCODED_ADDRS_LENGTH, HASH160_LEN,
-    MAX_STACKS_STRING_LEN, MAX_STRING_LEN, NUM_SUPPORTED_POST_CONDITIONS, STX_DECIMALS,
+    ClarityName, ContractName, PrincipalData, StacksAddress, C32_ENCODED_ADDRS_LENGTH, HASH160_LEN,
 };
 
 use crate::parser::c32;
 
-use super::value::{Value, ValueId, BIG_INT_SIZE};
-use crate::parser::ffi::fp_uint64_to_str;
+use super::value::{Value, ValueId};
 use crate::{check_canary, is_expert_mode, zxformat};
 
 pub const MAX_NUM_ARGS: u32 = 10;

app/rust/src/parser/utils.rs

@@ -0,0 +1,32 @@
+use super::ParserError;
+use nom::number::complete::{le_u16, le_u32, le_u64, le_u8};
+
+const FD_PREFIX: u8 = 0xfd;
+const FE_PREFIX: u8 = 0xfe;
+const FF_PREFIX: u8 = 0xff;
+
+// stick to the bitcoin spec
+// refer to https://learnmeabitcoin.com/technical/varint
+// for more detail
+pub fn read_varint(input: &[u8]) -> Result<(&[u8], u64), nom::Err<ParserError>> {
+    // read len prefix
+    let (rem, prefix) = le_u8(input)?;
+
+    // check prefix
+    match prefix {
+        FD_PREFIX => {
+            // the next 2-bytes are the next field len
+            le_u16(rem).map(|(rem, len)| (rem, len as u64))
+        }
+        FE_PREFIX => {
+            // the next 4-bytes are the next field len
+            le_u32(rem).map(|(rem, len)| (rem, len as u64))
+        }
+        FF_PREFIX => {
+            // the next 4-bytes are the next field len
+            le_u64(rem)
+        }
+        // the prefix is the actual len
+        _ => Ok((rem, prefix as _)),
+    }
+}