[0.7.0] refactor(borsh-rs): Avoid silent integer casts since they can…

… lead to hidden security issues (#75) Use `TryFrom` instead of `as u8` to make use of Rust strong type system. Casting integers is unsafe, we should not silently truncate the numbers. Also, in many of these cases, Rust will see that on the target platform `usize` is >= `u32` and should drop away those panic branches completely. # Test plan * `cargo test` passes without changes to the tests suite
near · Jun 17, 2020 · 64e8dc9 · 64e8dc9
1 parent a2508e6
commit 64e8dc9
Show file tree

Hide file tree

Showing 11 changed files with 50 additions and 25 deletions.
diff --git a/borsh-rs/CHANGELOG.md b/borsh-rs/CHANGELOG.md
@@ -0,0 +1,7 @@
+# 0.7.0
+
+- Extended `Box<T>` implementation for `?Sized` types (`[T]`, `str`, ...).
+- Added support for `std::borrow::Cow`
+- Avoid silent integer casts since they can lead to hidden security issues.
+- Removed `Cargo.lock` as it is advised for lib crates.
+
diff --git a/borsh-rs/borsh-derive-internal/Cargo.toml b/borsh-rs/borsh-derive-internal/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "borsh-derive-internal"
-version = "0.6.2"
+version = "0.7.0"
 authors = ["Near Inc <hello@nearprotocol.com>"]
 edition = "2018"
 license = "Apache-2.0"

diff --git a/borsh-rs/borsh-derive-internal/src/enum_de.rs b/borsh-rs/borsh-derive-internal/src/enum_de.rs
@@ -1,16 +1,19 @@
-use crate::attribute_helpers::{contains_initialize_with, contains_skip};
+use std::convert::TryFrom;
+
 use quote::quote;
 use syn::export::TokenStream2;
 use syn::{Fields, ItemEnum};
 
+use crate::attribute_helpers::{contains_initialize_with, contains_skip};
+
 pub fn enum_de(input: &ItemEnum) -> syn::Result<TokenStream2> {
     let name = &input.ident;
     let generics = &input.generics;
     let init_method = contains_initialize_with(&input.attrs)?;
     let mut variant_arms = TokenStream2::new();
     let mut deserializable_field_types = TokenStream2::new();
     for (variant_idx, variant) in input.variants.iter().enumerate() {
-        let variant_idx = variant_idx as u8;
+        let variant_idx = u8::try_from(variant_idx).expect("up to 256 enum variants are supported");
         let variant_ident = &variant.ident;
         let mut variant_header = TokenStream2::new();
         match &variant.fields {

diff --git a/borsh-rs/borsh-derive-internal/src/enum_ser.rs b/borsh-rs/borsh-derive-internal/src/enum_ser.rs
@@ -1,15 +1,18 @@
-use crate::attribute_helpers::contains_skip;
+use std::convert::TryFrom;
+
 use quote::quote;
 use syn::export::{Span, TokenStream2};
 use syn::{Fields, Ident, ItemEnum};
 
+use crate::attribute_helpers::contains_skip;
+
 pub fn enum_ser(input: &ItemEnum) -> syn::Result<TokenStream2> {
     let name = &input.ident;
     let generics = &input.generics;
     let mut body = TokenStream2::new();
     let mut serializable_field_types = TokenStream2::new();
     for (variant_idx, variant) in input.variants.iter().enumerate() {
-        let variant_idx = variant_idx as u8;
+        let variant_idx = u8::try_from(variant_idx).expect("up to 256 enum variants are supported");
         let variant_ident = &variant.ident;
         let mut variant_header = TokenStream2::new();
         let mut variant_body = TokenStream2::new();
@@ -22,7 +25,7 @@ pub fn enum_ser(input: &ItemEnum) -> syn::Result<TokenStream2> {
                         continue;
                     } else {
                         let field_type = &field.ty;
-                        serializable_field_types.extend(quote!{
+                        serializable_field_types.extend(quote! {
                             #field_type: borsh::ser::BorshSerialize,
                         });
                         variant_header.extend(quote! { #field_name, });
@@ -35,15 +38,16 @@ pub fn enum_ser(input: &ItemEnum) -> syn::Result<TokenStream2> {
             }
             Fields::Unnamed(fields) => {
                 for (field_idx, field) in fields.unnamed.iter().enumerate() {
-                    let field_idx = field_idx as u32;
+                    let field_idx =
+                        u32::try_from(field_idx).expect("up to 2^32 fields are supported");
                     if contains_skip(&field.attrs) {
                         let field_ident =
                             Ident::new(format!("_id{}", field_idx).as_str(), Span::call_site());
                         variant_header.extend(quote! { #field_ident, });
                         continue;
                     } else {
                         let field_type = &field.ty;
-                        serializable_field_types.extend(quote!{
+                        serializable_field_types.extend(quote! {
                             #field_type: borsh::ser::BorshSerialize,
                         });
 

diff --git a/borsh-rs/borsh-derive-internal/src/struct_ser.rs b/borsh-rs/borsh-derive-internal/src/struct_ser.rs
@@ -1,8 +1,11 @@
-use crate::attribute_helpers::contains_skip;
+use std::convert::TryFrom;
+
 use quote::quote;
 use syn::export::{Span, TokenStream2};
 use syn::{Fields, Index, ItemStruct};
 
+use crate::attribute_helpers::contains_skip;
+
 pub fn struct_ser(input: &ItemStruct) -> syn::Result<TokenStream2> {
     let name = &input.ident;
     let generics = &input.generics;
@@ -21,15 +24,15 @@ pub fn struct_ser(input: &ItemStruct) -> syn::Result<TokenStream2> {
                 body.extend(delta);
 
                 let field_type = &field.ty;
-                serializable_field_types.extend(quote!{
+                serializable_field_types.extend(quote! {
                     #field_type: borsh::ser::BorshSerialize,
                 });
             }
         }
         Fields::Unnamed(fields) => {
             for field_idx in 0..fields.unnamed.len() {
                 let field_idx = Index {
-                    index: field_idx as u32,
+                    index: u32::try_from(field_idx).expect("up to 2^32 fields are supported"),
                     span: Span::call_site(),
                 };
                 let delta = quote! {
@@ -112,4 +115,3 @@ mod tests {
         assert_eq(expected, actual);
     }
 }
-
diff --git a/borsh-rs/borsh-derive/Cargo.toml b/borsh-rs/borsh-derive/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "borsh-derive"
-version = "0.6.2"
+version = "0.7.0"
 authors = ["Near Inc <hello@nearprotocol.com>"]
 edition = "2018"
 license = "Apache-2.0"
@@ -16,7 +16,7 @@ Binary Object Representation Serializer for Hashing
 proc-macro = true
 
 [dependencies]
-borsh-derive-internal = { path = "../borsh-derive-internal" , version="0.6.2"}
-borsh-schema-derive-internal = { path = "../borsh-schema-derive-internal" , version="0.6.2"}
+borsh-derive-internal = { path = "../borsh-derive-internal" , version="0.7.0"}
+borsh-schema-derive-internal = { path = "../borsh-schema-derive-internal" , version="0.7.0"}
 syn = {version = "1", features = ["full", "fold"] }
 
diff --git a/borsh-rs/borsh-schema-derive-internal/Cargo.toml b/borsh-rs/borsh-schema-derive-internal/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "borsh-schema-derive-internal"
-version = "0.6.2"
+version = "0.7.0"
 authors = ["Near Inc <hello@nearprotocol.com>"]
 edition = "2018"
 license = "Apache-2.0"

diff --git a/borsh-rs/borsh/Cargo.toml b/borsh-rs/borsh/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "borsh"
-version = "0.6.2"
+version = "0.7.0"
 authors = ["Near Inc <hello@nearprotocol.com>"]
 edition = "2018"
 license = "Apache-2.0"
@@ -21,7 +21,7 @@ name = "generate_schema_schema"
 path = "src/generate_schema_schema.rs"
 
 [dependencies]
-borsh-derive = { path = "../borsh-derive", version = "0.6.2" }
+borsh-derive = { path = "../borsh-derive", version = "0.7.0" }
 
 [features]
 default = ["std"]

diff --git a/borsh-rs/borsh/src/de/hint.rs b/borsh-rs/borsh/src/de/hint.rs
@@ -1,7 +1,7 @@
 #[inline]
 pub fn cautious<T>(hint: u32) -> usize {
     let el_size = std::mem::size_of::<T>() as u32;
-    std::cmp::max(std::cmp::min(hint, 4096/el_size), 1u32) as _
+    std::cmp::max(std::cmp::min(hint, 4096 / el_size), 1) as usize
 }
 
 #[cfg(test)]

diff --git a/borsh-rs/borsh/src/de/mod.rs b/borsh-rs/borsh/src/de/mod.rs
@@ -224,7 +224,7 @@ where
         if len == 0 {
             Ok(Vec::new())
         } else if T::is_u8() && size_of::<T>() == size_of::<u8>() {
-            let len = len as usize;
+            let len = len.try_into().map_err(|_| io::ErrorKind::InvalidInput)?;
             if buf.len() < len {
                 return Err(io::Error::new(
                     io::ErrorKind::InvalidInput,
@@ -259,7 +259,7 @@ where
             let p = result.as_mut_ptr();
             unsafe {
                 forget(result);
-                let len = len as usize;
+                let len = len.try_into().map_err(|_| io::ErrorKind::InvalidInput)?;
                 let result = Vec::from_raw_parts(p, len, len);
                 Ok(result)
             }

diff --git a/borsh-rs/borsh/src/ser/mod.rs b/borsh-rs/borsh/src/ser/mod.rs
@@ -1,5 +1,6 @@
 use std::borrow::Cow;
 use std::collections::{BTreeMap, HashMap, HashSet};
+use std::convert::TryFrom;
 use std::mem::size_of;
 use std::{io, io::Write};
 
@@ -143,7 +144,9 @@ where
 {
     #[inline]
     fn serialize<W: Write>(&self, writer: &mut W) -> io::Result<()> {
-        writer.write_all(&(self.len() as u32).to_le_bytes())?;
+        writer.write_all(
+            &(u32::try_from(self.len()).map_err(|_| io::ErrorKind::InvalidInput)?).to_le_bytes(),
+        )?;
         if T::is_u8() && size_of::<T>() == size_of::<u8>() {
             // The code below uses unsafe memory representation from `&[T]` to `&[u8]`.
             // The size of the memory should match because `size_of::<T>() == size_of::<u8>()`.
@@ -190,7 +193,9 @@ where
     fn serialize<W: Write>(&self, writer: &mut W) -> io::Result<()> {
         let mut vec = self.iter().collect::<Vec<_>>();
         vec.sort_by(|a, b| a.partial_cmp(b).unwrap());
-        (vec.len() as u32).serialize(writer)?;
+        u32::try_from(vec.len())
+            .map_err(|_| io::ErrorKind::InvalidInput)?
+            .serialize(writer)?;
         for item in vec {
             item.serialize(writer)?;
         }
@@ -208,7 +213,9 @@ where
     fn serialize<W: Write>(&self, writer: &mut W) -> io::Result<()> {
         let mut vec = self.iter().collect::<Vec<_>>();
         vec.sort_by(|(a, _), (b, _)| a.partial_cmp(b).unwrap());
-        (vec.len() as u32).serialize(writer)?;
+        u32::try_from(vec.len())
+            .map_err(|_| io::ErrorKind::InvalidInput)?
+            .serialize(writer)?;
         for (key, value) in vec {
             key.serialize(writer)?;
             value.serialize(writer)?;
@@ -225,7 +232,9 @@ where
 {
     #[inline]
     fn serialize<W: Write>(&self, writer: &mut W) -> io::Result<()> {
-        (self.len() as u32).serialize(writer)?;
+        u32::try_from(self.len())
+            .map_err(|_| io::ErrorKind::InvalidInput)?
+            .serialize(writer)?;
         for (key, value) in self.iter() {
             key.serialize(writer)?;
             value.serialize(writer)?;