Skip to content

Commit

Permalink
Merge pull request #10 from tyshkor/develop
Browse files Browse the repository at this point in the history 
Audit fixes
  • Loading branch information
@zikriya
zikriya committed Jun 7, 2023
2 parents 1a372b7 + 54ecc2c commit 20ea2fd
Show file tree
Hide file tree
Showing 19 changed files with 1,046 additions and 1,460 deletions.
227 changes: 161 additions & 66 deletions contract/Cargo.lock
View file

Large diffs are not rendered by default.

10 changes: 5 additions & 5 deletions contract/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@ authors = ["CasperLabs <https://discord.com/invite/Q38s3Vh>"]
edition = "2021"

[dependencies]
casper-contract = "1.4.4"
casper-types = "1.5.0"
casper-contract = "3.0.0"
casper-types = "3.0.0"
contract-utils = { path = "../utils/contract-utils" }
k256 = { version = "0.7.3", default-features = false, features = ["ecdsa", "zeroize", "keccak256"] }
sha3 = "*"
hex = "0.4.3"
ecdsa = "0.10.2"
secp256k1 = {version = "0.19", features = ["recovery"] }
secp256k1 = {version = "0.27", features = ["recovery"] }
tiny-keccak = { version = "2.0", features = ["keccak"] }
getrandom = { version = "=0.2.7", features = ["js"]}

[[bin]]
name = "bridge_pool"
Expand All @@ -26,6 +26,6 @@ test = false

[features]
default = ["std"]
std = ["casper-contract/std", "casper-types/std"]
std = ["casper-contract/std"]

# default = []
110 changes: 95 additions & 15 deletions contract/src/bridge_pool_contract.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,20 @@ use crate::{
error::Error,
event::BridgePoolEvent,
};
use alloc::string::String;
use alloc::string::{String, ToString};
use alloc::vec::Vec;
use casper_contract::contract_api::runtime;
use casper_contract::unwrap_or_revert::UnwrapOrRevert;
use casper_types::{ContractPackageHash, U256};
use casper_types::RuntimeArgs;
use casper_types::{runtime_args, ContractPackageHash, U256};
use contract_utils::keccak::{keccak256, keccak256_hash};
use contract_utils::{ContractContext, ContractStorage};
use k256::ecdsa::{
recoverable::Signature as RecoverableSignature, signature::Signature as NonRecoverableSignature,
};
use secp256k1::{Message, Secp256k1};

const AMOUNT: &str = "amount";

pub trait BridgePoolContract<Storage: ContractStorage>: ContractContext<Storage> {
fn init(&mut self) {
Expand Down Expand Up @@ -98,6 +108,7 @@ pub trait BridgePoolContract<Storage: ContractStorage>: ContractContext<Storage>
amount: U256,
target_network: U256,
target_token: String,
target_address: String,
) -> Result<(), Error> {
let actor = detail::get_immediate_caller_address()
.unwrap_or_revert_with(Error::ImmediateCallerFail);
Expand All @@ -106,9 +117,7 @@ pub trait BridgePoolContract<Storage: ContractStorage>: ContractContext<Storage>
.map_err(|_| Error::NotContractPackageHash)?;

let bridge_pool_instance = BridgePool::instance();
bridge_pool_instance.swap(actor, token, target_token.clone(), amount, target_network)?;

let target_address = target_token;
bridge_pool_instance.swap(actor, token, target_token, amount, target_network)?;

self.emit(BridgePoolEvent::BridgeSwap {
actor,
Expand Down Expand Up @@ -137,13 +146,15 @@ pub trait BridgePoolContract<Storage: ContractStorage>: ContractContext<Storage>
}

// outer function to withdraw liquidity from the pool securely
#[allow(clippy::too_many_arguments)]
fn withdraw_signed(
&mut self,
token_address: String,
payee: String,
amount: U256,
chain_id: u64,
salt: String,
receiver: String,
signature: String,
) -> Result<(), Error> {
let actor = detail::get_immediate_caller_address()
Expand All @@ -152,25 +163,94 @@ pub trait BridgePoolContract<Storage: ContractStorage>: ContractContext<Storage>
let token = ContractPackageHash::from_formatted_str(token_address.as_str())
.map_err(|_| Error::NotContractPackageHash)?;

let salt_array: [u8; 32] = hex::decode(salt)
let bridge_pool_instance = BridgePool::instance();

let signature = hex::decode(signature).unwrap();

let salt: [u8; 32] = hex::decode(salt)
.map_err(|_| Error::SaltHexFail)?
.try_into()
.map_err(|_| Error::SaltWrongSize)?;
let signature_vec = hex::decode(signature).unwrap();

let bridge_pool_instance = BridgePool::instance();
let signer = bridge_pool_instance.withdraw_signed(
let message_hash = hex::encode(keccak256(
hex::encode(keccak256(
&[
token.to_formatted_string().as_bytes(),
payee.as_bytes(),
amount.to_string().as_bytes(),
receiver.as_bytes(),
&chain_id.to_be_bytes(),
&salt,
]
.concat()[..],
))
.as_bytes(),
));

let signature_rec = if signature.len() == 65 {
RecoverableSignature::from_bytes(&signature[..])
.map_err(|_| Error::RecoverableSignatureTryFromFail)?
} else {
NonRecoverableSignature::from_bytes(&signature[..])
.map_err(|_| Error::NonRecoverableSignatureTryFromFail)?
};

let hash =
&hex::decode(message_hash.clone()).map_err(|_| Error::MessageHashHexDecodingFail)?[..];
let sig = &signature_rec;

let s = Secp256k1::new();
let msg = Message::from_slice(hash).unwrap();
let mut sig_compact: Vec<u8> = sig.r().to_bytes().to_vec();
sig_compact.extend(&sig.s().to_bytes().to_vec());
let id_u8: u8 = From::from(sig.recovery_id());
let sig_v = secp256k1::ecdsa::RecoveryId::from_i32(id_u8 as i32).unwrap();
let rec_sig =
secp256k1::ecdsa::RecoverableSignature::from_compact(&sig_compact, sig_v).unwrap();
let pub_key = s.recover_ecdsa(&msg, &rec_sig).unwrap();
let public_key = Vec::from(&keccak256_hash(&pub_key.serialize_uncompressed()[1..])[12..]);

if bridge_pool_instance
.used_hashes_dict
.get::<bool>(message_hash.as_str())
.is_some()
{
return Err(Error::MessageAlreadyUsed);
} else {
bridge_pool_instance
.used_hashes_dict
.set(message_hash.as_str(), true);
}

let signer = hex::encode(public_key);

if !bridge_pool_instance
.signers_dict
.get::<bool>(&signer)
.ok_or(Error::NoValueInSignersDict)?
{
return Err(Error::InvalidSigner);
}

let client_addr = actor;
runtime::call_versioned_contract::<()>(
token,
payee.clone(),
None,
"transfer",
runtime_args! {
"recipient" => client_addr,
AMOUNT => amount
},
);
bridge_pool_instance.del_liquidity_generic_from_dict(
token.to_formatted_string(),
actor.as_account_hash().unwrap().to_string(),
amount,
chain_id,
salt_array,
signature_vec,
actor,
bridge_pool_instance.get_dict(actor)?,
)?;
self.emit(BridgePoolEvent::TransferBySignature {
signer,
receiver: payee,
receiver,
token,
amount,
});
Expand Down
115 changes: 72 additions & 43 deletions contract/src/data.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use casper_contract::{
};
use casper_types::RuntimeArgs;
use casper_types::{runtime_args, system::CallStackElement, ContractPackageHash, URef, U256};
use contract_utils::Dict;
use contract_utils::{keccak::keccak256, Dict};

use k256::ecdsa::{
recoverable::Signature as RecoverableSignature, signature::Signature as NonRecoverableSignature,
Expand Down Expand Up @@ -55,9 +55,9 @@ pub struct BridgePool {
allowed_targets_dict: Dict,
// dictionary to track used hashes
#[allow(unused)]
used_hashes_dict: Dict,
pub used_hashes_dict: Dict,
// dictionary to track signers
signers_dict: Dict,
pub signers_dict: Dict,
token_contract_package_hash_dict_name: Dict,
}

Expand Down Expand Up @@ -131,7 +131,7 @@ impl BridgePool {

let client_string: String = TryInto::try_into(client_address)?;
self.add_liquidity_generic(
token_contract_package_hash.to_string(),
token_contract_package_hash.to_formatted_string(),
client_string,
amount,
self.get_dict(client_address)?,
Expand Down Expand Up @@ -174,7 +174,7 @@ impl BridgePool {
let client_string: String = TryInto::try_into(client_address)?;
self.pay_from_me(token_contract_package_hash, client_address, amount);
self.del_liquidity_generic_from_dict(
token_contract_package_hash.to_string(),
token_contract_package_hash.to_formatted_string(),
client_string,
amount,
self.get_dict(client_address)?,
Expand All @@ -183,7 +183,7 @@ impl BridgePool {
}

// generic function to handle the case of a client and a contract when removing liquidity
fn del_liquidity_generic_from_dict(
pub fn del_liquidity_generic_from_dict(
&self,
token_contract_hash: String,
client: String,
Expand All @@ -207,24 +207,6 @@ impl BridgePool {
}
}

// withdraw liquidity from pool
pub fn withdraw(
&self,
token_contract_package_hash: ContractPackageHash,
client_address: Address,
amount: U256,
) -> Result<(), Error> {
let client_string: String = TryInto::try_into(client_address)?;
self.pay_from_me(token_contract_package_hash, client_address, amount);
self.del_liquidity_generic_from_dict(
token_contract_package_hash.to_string(),
client_string,
amount,
self.get_dict(client_address)?,
)?;
Ok(())
}

// function to add signer
pub fn add_signer(&self, signer: String) {
self.signers_dict.set(&signer, true)
Expand All @@ -251,19 +233,56 @@ impl BridgePool {
payee: String,
amount: U256,
chain_id: u64,
salt: [u8; 32],
salt_string: String,
token_recipient: String,
signature: alloc::vec::Vec<u8>,
actor: Address,
) -> Result<String, Error> {
let message_hash = contract_utils::keccak::message_hash(
token_contract_package_hash.to_formatted_string(),
payee,
amount.to_string(),
chain_id as i64,
salt,
);
let signer = self.signer_unique(message_hash, signature)?;
let signer_string = hex::encode(signer);
let salt: [u8; 32] = hex::decode(salt_string)
.map_err(|_| Error::SaltHexFail)?
.try_into()
.map_err(|_| Error::SaltWrongSize)?;

let message_hash = hex::encode(keccak256(
hex::encode(keccak256(
&[
token_contract_package_hash.to_formatted_string().as_bytes(),
payee.as_bytes(),
amount.to_string().as_bytes(),
token_recipient.as_bytes(),
&chain_id.to_be_bytes(),
&salt,
]
.concat()[..],
))
.as_bytes(),
));

let signature_rec = if signature.len() == 65 {
RecoverableSignature::from_bytes(&signature[..])
.map_err(|_| Error::RecoverableSignatureTryFromFail)?
} else {
NonRecoverableSignature::from_bytes(&signature[..])
.map_err(|_| Error::NonRecoverableSignatureTryFromFail)?
};

let public_key = contract_utils::keccak::ecdsa_recover(
&hex::decode(message_hash.clone()).map_err(|_| Error::MessageHashHexDecodingFail)?[..],
&signature_rec,
)
.map_err(|_| Error::EcdsaPublicKeyRecoveryFail)?;

if self
.used_hashes_dict
.get::<bool>(message_hash.as_str())
.is_some()
{
return Err(Error::MessageAlreadyUsed);
} else {
self.used_hashes_dict.set(message_hash.as_str(), true);
}

let signer_string = hex::encode(public_key);

if !self
.signers_dict
Expand All @@ -272,9 +291,19 @@ impl BridgePool {
{
return Err(Error::InvalidSigner);
}
self.pay_from_me(token_contract_package_hash, actor, amount);

let client_addr = actor;
runtime::call_versioned_contract::<()>(
token_contract_package_hash,
None,
"transfer",
runtime_args! {
"recipient" => client_addr,
AMOUNT => amount
},
);
self.del_liquidity_generic_from_dict(
token_contract_package_hash.to_string(),
token_contract_package_hash.to_formatted_string(),
actor.as_account_hash().unwrap().to_string(),
amount,
self.get_dict(actor)?,
Expand Down Expand Up @@ -319,12 +348,11 @@ impl BridgePool {
.map_err(|_| Error::NonRecoverableSignatureTryFromFail)?
};

let message_hash_bytes =
hex::decode(message_hash.clone()).map_err(|_| Error::MessageHashHexDecodingFail)?;

let public_key =
contract_utils::keccak::ecdsa_recover(&message_hash_bytes[..], &signature_rec)
.map_err(|_| Error::EcdsaPublicKeyRecoveryFail)?;
let public_key = contract_utils::keccak::ecdsa_recover(
&hex::decode(message_hash.clone()).map_err(|_| Error::MessageHashHexDecodingFail)?[..],
&signature_rec,
)
.map_err(|_| Error::EcdsaPublicKeyRecoveryFail)?;

if self
.used_hashes_dict
Expand Down Expand Up @@ -468,7 +496,7 @@ impl BridgePool {
runtime::call_versioned_contract::<()>(token, None, "transfer", args);
}

fn get_dict(&self, client_address: Address) -> Result<&Dict, Error> {
pub fn get_dict(&self, client_address: Address) -> Result<&Dict, Error> {
match client_address {
Address::Account(_) => Ok(&self.account_hash_liquidities_dict),
Address::ContractPackage(_) => Ok(&self.hash_addr_liquidities_dict),
Expand Down Expand Up @@ -555,6 +583,7 @@ pub fn emit(event: &BridgePoolEvent) {
events.push(param);
}
};

for param in events {
let _: URef = storage::new_uref(param);
}
Expand Down
1 change: 1 addition & 0 deletions contract/src/error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ pub enum Error {
PublicKeyTryIntoFail = 39,
ImmediateCallerFail = 40,
SignerWrongFormat = 41,
MessageHashNotEqualToGenerated = 42,
}

impl From<Error> for ApiError {
Expand Down
Loading

0 comments on commit 20ea2fd

Please sign in to comment.