Skip to content

Commit

Permalink
tighen the allowed usage of Hash
Browse files Browse the repository at this point in the history
  • Loading branch information
@jayz22
jayz22 committed Apr 17, 2024
1 parent c5550c6 commit f394cfe
Show file tree
Hide file tree
Showing 7 changed files with 63 additions and 55 deletions.
9 changes: 8 additions & 1 deletion soroban-sdk-macros/src/derive_enum.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ pub fn derive_type_enum(
into_xdr,
} = map_tuple_variant(
path,
&vis,
enum_ident,
&case_num_lit,
&case_name_str_lit,
Expand Down Expand Up @@ -333,6 +334,7 @@ fn map_empty_variant(

fn map_tuple_variant(
path: &Path,
vis: &Visibility,
enum_ident: &Ident,
case_num_lit: &Literal,
case_name_str_lit: &Literal,
Expand All @@ -343,9 +345,14 @@ fn map_tuple_variant(
errors: &mut Vec<Error>,
) -> VariantTokens {
let spec_case = {
let allow_hash = if let Visibility::Public(_) = vis {
false
} else {
true
};
let field_types = fields
.iter()
.map(|f| match map_type(&f.ty) {
.map(|f| match map_type(&f.ty, allow_hash) {
Ok(t) => t,
Err(e) => {
errors.push(e);
Expand Down
37 changes: 14 additions & 23 deletions soroban-sdk-macros/src/derive_fn.rs
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::map_type::map_type;
use itertools::MultiUnzip;
use proc_macro2::TokenStream as TokenStream2;
use quote::{format_ident, quote};
Expand All @@ -6,22 +7,31 @@ use syn::{
punctuated::Punctuated,
spanned::Spanned,
token::{Colon, Comma},
Attribute, Error, FnArg, Ident, Pat, PatIdent, PatType, Path, Type, TypePath, TypeReference,
Attribute, Error, FnArg, Ident, Pat, PatIdent, PatType, Path, ReturnType, Type, TypePath,
TypeReference,
};

#[allow(clippy::too_many_arguments)]
pub fn derive_fn(
pub fn derive_pub_fn(
crate_path: &Path,
call: &TokenStream2,
ident: &Ident,
attrs: &[Attribute],
inputs: &Punctuated<FnArg, Comma>,
output: &ReturnType,
trait_ident: Option<&Ident>,
client_ident: &str,
) -> Result<TokenStream2, TokenStream2> {
// Collect errors as they are encountered and emit them at the end.
let mut errors = Vec::<Error>::new();

let allow_hash = ident == "__check_auth";
if let ReturnType::Type(_, ty) = output {
if let Err(e) = map_type(ty, allow_hash) {
errors.push(e);
}
}

// Prepare the env input.
let env_input = inputs.first().and_then(|a| match a {
FnArg::Typed(pat_type) => {
Expand Down Expand Up @@ -55,28 +65,9 @@ pub fn derive_fn(
.enumerate()
.map(|(i, a)| match a {
FnArg::Typed(pat_ty) => {
if ident != "__check_auth" {
let mut ty = &*pat_ty.ty;
if let Type::Reference(TypeReference { elem, .. }) = ty {
ty = elem;
}
if let Type::Path(TypePath {
path: syn::Path { segments, .. },
..
}) = ty
{
if segments.last().map_or(false, |s| s.ident == "Hash" && !s.arguments.is_none()) {
errors.push(Error::new(a.span(), "`Hash<T>` cannot be used as argument to a public user function,
since there is no guarantee the received input is from a secure hash function.
If you still intend to use a hash with such a guarantee, please use `ByteN`"));
} else {
()
}
} else {
()
}
if let Err(e) = map_type(&pat_ty.ty, allow_hash) {
errors.push(e);
}

let ident = format_ident!("arg_{}", i);
let arg = FnArg::Typed(PatType {
attrs: vec![],
Expand Down
13 changes: 2 additions & 11 deletions soroban-sdk-macros/src/derive_spec_fn.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ pub fn derive_fn_spec(
errors.push(Error::new(a.span(), "argument not supported"));
"".to_string()
};
match map_type(&pat_type.ty) {
match map_type(&pat_type.ty, ident == "__check_auth") {
Ok(type_) => {
let name = name.try_into().unwrap_or_else(|_| {
const MAX: u32 = 30;
Expand All @@ -71,15 +71,6 @@ pub fn derive_fn_spec(
));
StringM::<MAX>::default()
});

if let ScSpecTypeDef::Hash(_) = type_ {
if ident != "__check_auth" {
errors.push(Error::new(a.span(), "`Hash<T>` cannot be used as argument to a public user function,
since there is no guarantee the received input is from a secure hash function.
If you still intend to use a hash with such a guarantee, please use `ByteN`"));
}
}

ScSpecFunctionInputV0 {
doc: "".try_into().unwrap(),
name,
Expand Down Expand Up @@ -109,7 +100,7 @@ pub fn derive_fn_spec(

// Prepare the output.
let spec_result = match output {
ReturnType::Type(_, ty) => vec![match map_type(ty) {
ReturnType::Type(_, ty) => vec![match map_type(ty, false) {
Ok(spec) => spec,
Err(e) => {
errors.push(e);
Expand Down
8 changes: 6 additions & 2 deletions soroban-sdk-macros/src/derive_struct.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,11 @@ pub fn derive_type_struct(
) -> TokenStream2 {
// Collect errors as they are encountered and emit them at the end.
let mut errors = Vec::<Error>::new();

let allow_hash = if let Visibility::Public(_) = vis {
false
} else {
true
};
let fields = &data.fields;
let field_count_usize: usize = fields.len();
let (spec_fields, field_idents, field_names, field_idx_lits, try_from_xdrs, try_into_xdrs): (Vec<_>, Vec<_>, Vec<_>, Vec<_>, Vec<_>, Vec<_>) = fields
Expand All @@ -43,7 +47,7 @@ pub fn derive_type_struct(
errors.push(Error::new(field_ident.span(), format!("struct field name is too long: {}, max is {MAX}", field_name.len())));
StringM::<MAX>::default()
}),
type_: match map_type(&field.ty) {
type_: match map_type(&field.ty, allow_hash) {
Ok(t) => t,
Err(e) => {
errors.push(e);
Expand Down
7 changes: 6 additions & 1 deletion soroban-sdk-macros/src/derive_struct_tuple.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ pub fn derive_type_struct_tuple(

let fields = &data.fields;
let field_count_usize: usize = fields.len();
let allow_hash = if let Visibility::Public(_) = vis {
false
} else {
true
};

let (field_specs, field_idx_lits, try_from_xdrs, try_into_xdrs): (Vec<_>, Vec<_>, Vec<_>, Vec<_>) = fields
.iter()
Expand All @@ -36,7 +41,7 @@ pub fn derive_type_struct_tuple(
let field_spec = ScSpecUdtStructFieldV0 {
doc: docs_from_attrs(&field.attrs).try_into().unwrap(), // TODO: Truncate docs, or display friendly compile error.
name: field_name.try_into().unwrap_or_else(|_| StringM::default()),
type_: match map_type(&field.ty) {
type_: match map_type(&field.ty, allow_hash) {
Ok(t) => t,
Err(e) => {
errors.push(e);
Expand Down
5 changes: 3 additions & 2 deletions soroban-sdk-macros/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use derive_client::{derive_client_impl, derive_client_type};
use derive_enum::derive_type_enum;
use derive_enum_int::derive_type_enum_int;
use derive_error_enum_int::derive_type_error_enum_int;
use derive_fn::{derive_contract_function_registration_ctor, derive_fn};
use derive_fn::{derive_contract_function_registration_ctor, derive_pub_fn};
use derive_spec_fn::derive_fn_spec;
use derive_struct::derive_type_struct;
use derive_struct_tuple::derive_type_struct_tuple;
Expand Down Expand Up @@ -231,12 +231,13 @@ pub fn contractimpl(metadata: TokenStream, input: TokenStream) -> TokenStream {
.map(|m| {
let ident = &m.sig.ident;
let call = quote! { <super::#ty>::#ident };
derive_fn(
derive_pub_fn(
&crate_path,
&call,
ident,
&m.attrs,
&m.sig.inputs,
&m.sig.output,
trait_ident,
&client_ident,
)
Expand Down
39 changes: 24 additions & 15 deletions soroban-sdk-macros/src/map_type.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use syn::{
};

#[allow(clippy::too_many_lines)]
pub fn map_type(t: &Type) -> Result<ScSpecTypeDef, Error> {
pub fn map_type(t: &Type, allow_hash: bool) -> Result<ScSpecTypeDef, Error> {
match t {
Type::Path(TypePath {
qself: None,
Expand Down Expand Up @@ -61,8 +61,8 @@ pub fn map_type(t: &Type) -> Result<ScSpecTypeDef, Error> {
))?,
};
Ok(ScSpecTypeDef::Result(Box::new(ScSpecTypeResult {
ok_type: Box::new(map_type(ok)?),
error_type: Box::new(map_type(err)?),
ok_type: Box::new(map_type(ok, allow_hash)?),
error_type: Box::new(map_type(err, allow_hash)?),
})))
}
"Option" => {
Expand All @@ -74,7 +74,7 @@ pub fn map_type(t: &Type) -> Result<ScSpecTypeDef, Error> {
))?,
};
Ok(ScSpecTypeDef::Option(Box::new(ScSpecTypeOption {
value_type: Box::new(map_type(t)?),
value_type: Box::new(map_type(t, allow_hash)?),
})))
}
"Vec" => {
Expand All @@ -86,7 +86,7 @@ pub fn map_type(t: &Type) -> Result<ScSpecTypeDef, Error> {
))?,
};
Ok(ScSpecTypeDef::Vec(Box::new(ScSpecTypeVec {
element_type: Box::new(map_type(t)?),
element_type: Box::new(map_type(t, allow_hash)?),
})))
}
"Map" => {
Expand All @@ -98,8 +98,8 @@ pub fn map_type(t: &Type) -> Result<ScSpecTypeDef, Error> {
))?,
};
Ok(ScSpecTypeDef::Map(Box::new(ScSpecTypeMap {
key_type: Box::new(map_type(k)?),
value_type: Box::new(map_type(v)?),
key_type: Box::new(map_type(k, allow_hash)?),
value_type: Box::new(map_type(v, allow_hash)?),
})))
}
"BytesN" => {
Expand All @@ -113,14 +113,21 @@ pub fn map_type(t: &Type) -> Result<ScSpecTypeDef, Error> {
Ok(ScSpecTypeDef::BytesN(ScSpecTypeBytesN { n }))
}
"Hash" => {
let n = match args.as_slice() {
[GenericArgument::Const(Expr::Lit(ExprLit { lit: Lit::Int(int), .. }))] => int.base10_parse()?,
[..] => Err(Error::new(
if allow_hash {
let n = match args.as_slice() {
[GenericArgument::Const(Expr::Lit(ExprLit { lit: Lit::Int(int), .. }))] => int.base10_parse()?,
[..] => Err(Error::new(
t.span(),
"incorrect number of generic arguments, expect one for BytesN<N>",
))?,
};
Ok(ScSpecTypeDef::Hash(ScSpectTypeHash { n }))
} else {
Err(Error::new(
t.span(),
"incorrect number of generic arguments, expect one for BytesN<N>",
))?,
};
Ok(ScSpecTypeDef::Hash(ScSpectTypeHash { n }))
"Hash<N> cannot be used in contexts where there is no guarantee it comes from a secure hash function",
))
}
}
_ => Err(Error::new(
angle_bracketed.span(),
Expand All @@ -132,10 +139,12 @@ pub fn map_type(t: &Type) -> Result<ScSpecTypeDef, Error> {
}
}
Type::Tuple(TypeTuple { elems, .. }) => {
let map_type_reject_hash =
|t: &Type| -> Result<ScSpecTypeDef, Error> { map_type(t, false) };
Ok(ScSpecTypeDef::Tuple(Box::new(ScSpecTypeTuple {
value_types: elems
.iter()
.map(map_type)
.map(map_type_reject_hash)
.collect::<Result<Vec<ScSpecTypeDef>, Error>>()? // TODO: Implement conversion to VecM from iters to omit this collect.
.try_into()
.map_err(|e| {
Expand Down

0 comments on commit f394cfe

Please sign in to comment.