Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Address audit issues #5

Merged
merged 14 commits into from
Nov 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitmodules
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,6 @@
[submodule "solidity/lib/openzeppelin-contracts-upgradeable"]
path = solidity/lib/openzeppelin-contracts-upgradeable
url = https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable
[submodule "solidity/lib/skip-go-evm-contracts"]
path = solidity/lib/skip-go-evm-contracts
url = https://github.com/skip-mev/skip-go-evm-contracts
8 changes: 8 additions & 0 deletions cosmwasm/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions cosmwasm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ members = [
"contracts/fast-transfer-gateway",
"contracts/cw7683",
"packages/*",
"bin/print-order-id",
]

[profile.release]
Expand Down
8 changes: 8 additions & 0 deletions cosmwasm/bin/print-order-id/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[package]
name = "print-order-id"
version = "0.1.0"
edition = "2021"

[dependencies]
cosmwasm-std = { workspace = true }
go-fast = { workspace = true }
19 changes: 19 additions & 0 deletions cosmwasm/bin/print-order-id/src/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
use cosmwasm_std::{HexBinary, Uint128};
use go_fast::{helpers::keccak256_hash, FastTransferOrder};

fn main() {
let order = FastTransferOrder {
sender: keccak256_hash("order_sender".as_bytes()),
recipient: keccak256_hash("order_recipient".as_bytes()),
amount_in: Uint128::new(1_000000),
amount_out: Uint128::new(2_000000),
nonce: 5,
source_domain: 1,
destination_domain: 2,
timeout_timestamp: 1234567890,
data: Some(HexBinary::from("order_data".as_bytes())),
};

println!("== Output ==");
println!("{}", order.id());
}
6 changes: 6 additions & 0 deletions cosmwasm/contracts/fast-transfer-gateway/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@ pub enum ContractError {

#[error("Incorrect domain for settlement")]
IncorrectDomainForSettlement,

#[error("Order recipient cannot be mailbox")]
OrderRecipientCannotBeMailbox,

#[error("Duplicate order")]
DuplicateOrder,
}

pub type ContractResult<T> = Result<T, ContractError>;
Expand Down
9 changes: 9 additions & 0 deletions cosmwasm/contracts/fast-transfer-gateway/src/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ pub fn fill_order(

let recipient_address = bech32_encode(&config.address_prefix, &order.recipient)?;

if recipient_address == config.mailbox_addr {
return Err(ContractError::OrderRecipientCannotBeMailbox);
}

let msg: CosmosMsg = match order.data {
Some(data) => WasmMsg::Execute {
contract_addr: recipient_address.clone().to_string(),
Expand Down Expand Up @@ -96,6 +100,10 @@ pub fn initiate_settlement(
return Err(ContractError::Unauthorized);
}

if fills_to_settle.contains(&order_fill) {
return Err(ContractError::DuplicateOrder);
}

fills_to_settle.push(order_fill);
}

Expand Down Expand Up @@ -145,6 +153,7 @@ pub fn initiate_timeout(
for order in &orders {
assert_order_is_expired(&env, order)?;
assert_order_not_filled(deps.as_ref(), order.id())?;
assert_local_domain(deps.as_ref(), order.destination_domain)?;
}

let order_ids = orders
Expand Down
6 changes: 1 addition & 5 deletions cosmwasm/contracts/fast-transfer-gateway/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,8 @@ pub fn assert_order_is_expired(env: &Env, order: &FastTransferOrder) -> Contract
}

pub fn assert_order_is_not_expired(env: &Env, order: &FastTransferOrder) -> ContractResult<()> {
if order.timeout_timestamp == 0 {
return Ok(());
}

let timeout_timestamp = Timestamp::from_seconds(order.timeout_timestamp);
if env.block.time.seconds() > timeout_timestamp.seconds() {
if env.block.time.seconds() >= timeout_timestamp.seconds() {
return Err(ContractError::OrderTimedOut);
}

Expand Down
18 changes: 16 additions & 2 deletions cosmwasm/contracts/fast-transfer-gateway/tests/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@ use cosmwasm_std::{
};
use go_fast::{
gateway::{Config, ExecuteMsg},
helpers::keccak256_hash,
FastTransferOrder,
};
use go_fast_transfer_cw::{
error::ContractResponse,
helpers::bech32_encode,
state::{CONFIG, LOCAL_DOMAIN, NONCE, REMOTE_DOMAINS},
};
use hyperlane::mailbox::{DefaultHookResponse, QueryMsg as HplQueryMsg, RequiredHookResponse};
Expand All @@ -28,7 +30,12 @@ pub fn default_instantiate() -> (OwnedDeps<MemoryStorage, MockApi, MockQuerier>,
&Config {
token_denom: "uusdc".to_string(),
address_prefix: "osmo".to_string(),
mailbox_addr: "mailbox_contract_address".into(),
mailbox_addr: bech32_encode(
"osmo",
&keccak256_hash("mailbox_contract_address".as_bytes()),
)
.unwrap()
.into_string(),
hook_addr: "hook_contract_address".into(),
},
)
Expand All @@ -50,7 +57,14 @@ pub fn default_instantiate() -> (OwnedDeps<MemoryStorage, MockApi, MockQuerier>,
let wasm_handler = |query: &WasmQuery| -> QuerierResult {
match query {
WasmQuery::Smart { contract_addr, msg } => {
if contract_addr == "mailbox_contract_address" {
if contract_addr
== &bech32_encode(
"osmo",
&keccak256_hash("mailbox_contract_address".as_bytes()),
)
.unwrap()
.into_string()
{
let msg: HplQueryMsg = from_json(msg).unwrap();
match msg {
HplQueryMsg::Hook(_) => todo!(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ use go_fast::{
gateway::{ExecuteMsg, OrderFill, QueryMsg},
FastTransferOrder,
};
use go_fast_transfer_cw::helpers::{bech32_decode, left_pad_bytes};
use go_fast_transfer_cw::{
helpers::{bech32_decode, left_pad_bytes},
state::CONFIG,
};

pub mod common;

Expand Down Expand Up @@ -167,6 +170,55 @@ fn test_fill_order_with_data() {
);
}

#[test]
fn test_fill_order_fails_when_order_recipient_is_mailbox() {
let (mut deps, env) = default_instantiate();

let user_address = deps.api.with_prefix("osmo").addr_make("user");

let test_payload = to_json_binary(&BankMsg::Send {
to_address: "solver".to_string(),
amount: vec![coin(98_000_000, "uusdc")],
})
.unwrap();

let mailbox_address = CONFIG.load(deps.as_ref().storage).unwrap().mailbox_addr;

let order = FastTransferOrder {
sender: HexBinary::from(left_pad_bytes(
bech32_decode(user_address.as_str()).unwrap(),
32,
)),
recipient: HexBinary::from(left_pad_bytes(
bech32_decode(mailbox_address.as_str()).unwrap(),
32,
)),
amount_in: Uint128::new(100_000_000),
amount_out: Uint128::new(98_000_000),
nonce: 1,
source_domain: 2,
destination_domain: 1,
timeout_timestamp: env.block.time.seconds() + 1000,
data: Some(HexBinary::from(test_payload.clone())),
};

let execute_msg = ExecuteMsg::FillOrder {
filler: Addr::unchecked("solver"),
order: order.clone(),
};

let res = go_fast_transfer_cw::contract::execute(
deps.as_mut(),
env.clone(),
mock_info("solver", &[coin(order.amount_out.u128(), "uusdc")]),
execute_msg.clone(),
)
.unwrap_err()
.to_string();

assert_eq!(res, "Order recipient cannot be mailbox");
}

#[test]
fn test_fill_order_fails_on_incorrect_amount() {
let (mut deps, env) = default_instantiate();
Expand Down Expand Up @@ -470,3 +522,44 @@ fn test_fill_order_fails_on_timed_out_order() {

assert_eq!(res, "Order timed out");
}

#[test]
fn test_fill_order_fails_on_timed_out_order_exact() {
let (mut deps, env) = default_instantiate();

let user_address = deps.api.with_prefix("osmo").addr_make("user");

let order = FastTransferOrder {
sender: HexBinary::from(left_pad_bytes(
bech32_decode(user_address.as_str()).unwrap(),
32,
)),
recipient: HexBinary::from(left_pad_bytes(
bech32_decode(user_address.as_str()).unwrap(),
32,
)),
amount_in: Uint128::new(100_000_000),
amount_out: Uint128::new(98_000_000),
nonce: 1,
source_domain: 2,
destination_domain: 1,
timeout_timestamp: env.block.time.seconds(),
data: None,
};

let execute_msg = ExecuteMsg::FillOrder {
filler: Addr::unchecked("solver"),
order: order.clone(),
};

let res = go_fast_transfer_cw::contract::execute(
deps.as_mut(),
env.clone(),
mock_info("solver", &[coin(order.amount_out.u128(), "uusdc")]),
execute_msg.clone(),
)
.unwrap_err()
.to_string();

assert_eq!(res, "Order timed out");
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use common::default_instantiate;
use cosmwasm_std::{testing::mock_info, to_json_binary, Addr, HexBinary, ReplyOn, SubMsg, WasmMsg};
use go_fast::gateway::ExecuteMsg;
use go_fast::{gateway::ExecuteMsg, helpers::keccak256_hash};
use go_fast_transfer_cw::{
helpers::{bech32_decode, left_pad_bytes},
helpers::{bech32_decode, bech32_encode, left_pad_bytes},
state::{self, REMOTE_DOMAINS},
};
use hyperlane::mailbox::{DispatchMsg, ExecuteMsg as MailboxExecuteMsg};
Expand Down Expand Up @@ -45,7 +45,12 @@ fn test_initiate_settlement() {
SubMsg {
id: 0,
msg: WasmMsg::Execute {
contract_addr: "mailbox_contract_address".into(),
contract_addr: bech32_encode(
"osmo",
&keccak256_hash("mailbox_contract_address".as_bytes()),
)
.unwrap()
.into_string(),
msg: to_json_binary(&MailboxExecuteMsg::Dispatch(DispatchMsg {
dest_domain: 2,
recipient_addr: HexBinary::from_hex(
Expand Down Expand Up @@ -115,7 +120,12 @@ fn test_initiate_settlement_multiple_orders() {
SubMsg {
id: 0,
msg: WasmMsg::Execute {
contract_addr: "mailbox_contract_address".into(),
contract_addr: bech32_encode(
"osmo",
&keccak256_hash("mailbox_contract_address".as_bytes()),
)
.unwrap()
.into_string(),
msg: to_json_binary(&MailboxExecuteMsg::Dispatch(DispatchMsg {
dest_domain: 2,
recipient_addr: HexBinary::from_hex(
Expand Down Expand Up @@ -307,3 +317,37 @@ fn test_initiate_settlement_multiple_orders_fails_if_source_domains_are_differen

assert_eq!(res, "Source domains must match");
}

#[test]
fn test_initiate_settlement_fails_if_orders_are_duplicate() {
let (mut deps, env) = default_instantiate();

let solver_address = deps.api.with_prefix("osmo").addr_make("solver");

let order_id = HexBinary::from_hex("1234").unwrap();

state::order_fills()
.create_order_fill(
deps.as_mut().storage,
order_id.clone(),
solver_address.clone(),
2,
)
.unwrap();

let execute_msg = ExecuteMsg::InitiateSettlement {
order_ids: vec![order_id.clone(), order_id.clone()],
repayment_address: HexBinary::from(left_pad_bytes(
bech32_decode(solver_address.as_str()).unwrap(),
32,
)),
};

let info = mock_info(solver_address.as_str(), &[]);

let res = go_fast_transfer_cw::contract::execute(deps.as_mut(), env, info, execute_msg.clone())
.unwrap_err()
.to_string();

assert_eq!(res, "Duplicate order");
}
Loading
Loading