diff --git a/contracts/cw20-escrow/schema/details_response.json b/contracts/cw20-escrow/schema/details_response.json index 653de5dde..89b9ae6fc 100644 --- a/contracts/cw20-escrow/schema/details_response.json +++ b/contracts/cw20-escrow/schema/details_response.json @@ -5,6 +5,7 @@ "required": [ "arbiter", "cw20_balance", + "cw20_whitelist", "id", "native_balance", "recipient", @@ -26,6 +27,13 @@ "$ref": "#/definitions/Cw20CoinHuman" } }, + "cw20_whitelist": { + "description": "Whitelisted cw20 tokens", + "type": "array", + "items": { + "$ref": "#/definitions/HumanAddr" + } + }, "end_height": { "description": "When end height set and block height exceeds this value, the escrow is expired. Once an escrow is expired, it can be returned to the original funder (via \"refund\").", "type": [ diff --git a/contracts/cw20-escrow/schema/handle_msg.json b/contracts/cw20-escrow/schema/handle_msg.json index 881c52497..a32f24b84 100644 --- a/contracts/cw20-escrow/schema/handle_msg.json +++ b/contracts/cw20-escrow/schema/handle_msg.json @@ -109,6 +109,16 @@ } ] }, + "cw20_whitelist": { + "description": "Besides any possible tokens sent with the CreateMsg, this is a list of all cw20 token addresses that are accepted by the escrow during a top-up. This is required to avoid a DoS attack by topping-up with an invalid cw20 contract. See https://github.com/CosmWasm/cosmwasm-plus/issues/19", + "type": [ + "array", + "null" + ], + "items": { + "$ref": "#/definitions/HumanAddr" + } + }, "end_height": { "description": "When end height set and block height exceeds this value, the escrow is expired. Once an escrow is expired, it can be returned to the original funder (via \"refund\").", "type": [ diff --git a/contracts/cw20-escrow/schema/receive_msg.json b/contracts/cw20-escrow/schema/receive_msg.json index 43e06fe81..eae75fb94 100644 --- a/contracts/cw20-escrow/schema/receive_msg.json +++ b/contracts/cw20-escrow/schema/receive_msg.json @@ -51,6 +51,16 @@ } ] }, + "cw20_whitelist": { + "description": "Besides any possible tokens sent with the CreateMsg, this is a list of all cw20 token addresses that are accepted by the escrow during a top-up. This is required to avoid a DoS attack by topping-up with an invalid cw20 contract. See https://github.com/CosmWasm/cosmwasm-plus/issues/19", + "type": [ + "array", + "null" + ], + "items": { + "$ref": "#/definitions/HumanAddr" + } + }, "end_height": { "description": "When end height set and block height exceeds this value, the escrow is expired. Once an escrow is expired, it can be returned to the original funder (via \"refund\").", "type": [ diff --git a/contracts/cw20-escrow/src/contract.rs b/contracts/cw20-escrow/src/contract.rs index c5bc2ef13..a64ea7d37 100644 --- a/contracts/cw20-escrow/src/contract.rs +++ b/contracts/cw20-escrow/src/contract.rs @@ -74,6 +74,7 @@ pub fn try_create( // there are native coins sent with the message native_balance: env.message.sent_funds, cw20_balance: vec![], + cw20_whitelist: msg.canonical_whitelist(&deps.api)?, }; // try to store it, fail if the id was already in use @@ -93,6 +94,12 @@ pub fn try_cw20_create( token: Cw20Coin, msg: CreateMsg, ) -> StdResult { + let mut cw20_whitelist = msg.canonical_whitelist(&deps.api)?; + // make sure the token sent is on the whitelist by default + if !cw20_whitelist.iter().any(|t| t == &token.address) { + cw20_whitelist.push(token.address.clone()) + } + let escrow = Escrow { arbiter: deps.api.canonical_address(&msg.arbiter)?, recipient: deps.api.canonical_address(&msg.recipient)?, @@ -102,6 +109,7 @@ pub fn try_cw20_create( // there are native coins sent with the message native_balance: vec![], cw20_balance: vec![token], + cw20_whitelist, }; // try to store it, fail if the id was already in use @@ -142,6 +150,13 @@ pub fn try_cw20_top_up( // this fails is no escrow there let mut escrow = escrows_read(&deps.storage).load(id.as_bytes())?; + // ensure the token is on the whitelist + if !escrow.cw20_whitelist.iter().any(|t| t == &token.address) { + return Err(StdError::generic_err( + "Only accepts tokens on the cw20_whitelist", + )); + } + // combine these two add_cw20_token(&mut escrow.cw20_balance, token); // and save @@ -298,6 +313,8 @@ fn query_details( ) -> StdResult { let escrow = escrows_read(&deps.storage).load(id.as_bytes())?; + let cw20_whitelist = escrow.human_whitelist(&deps.api)?; + // transform tokens let cw20_balance: StdResult> = escrow .cw20_balance @@ -319,6 +336,7 @@ fn query_details( end_time: escrow.end_time, native_balance: escrow.native_balance, cw20_balance: cw20_balance?, + cw20_whitelist, }; Ok(details) } @@ -354,7 +372,8 @@ mod tests { arbiter: HumanAddr::from("arbitrate"), recipient: HumanAddr::from("recd"), end_time: None, - end_height: None, + end_height: Some(123456), + cw20_whitelist: None, }; let sender = HumanAddr::from("source"); let balance = coins(100, "tokens"); @@ -363,6 +382,23 @@ mod tests { assert_eq!(0, res.messages.len()); assert_eq!(log("action", "create"), res.log[0]); + // ensure the details is what we expect + let details = query_details(&deps, "foobar".to_string()).unwrap(); + assert_eq!( + details, + DetailsResponse { + id: "foobar".to_string(), + arbiter: HumanAddr::from("arbitrate"), + recipient: HumanAddr::from("recd"), + source: HumanAddr::from("source"), + end_height: Some(123456), + end_time: None, + native_balance: balance.clone(), + cw20_balance: vec![], + cw20_whitelist: vec![], + } + ); + // approve it let id = create.id.clone(); let env = mock_env(&deps.api, &create.arbiter, &[]); @@ -405,6 +441,7 @@ mod tests { recipient: HumanAddr::from("recd"), end_time: None, end_height: None, + cw20_whitelist: Some(vec![HumanAddr::from("other-token")]), }; let receive = Cw20ReceiveMsg { sender: HumanAddr::from("source"), @@ -417,6 +454,29 @@ mod tests { assert_eq!(0, res.messages.len()); assert_eq!(log("action", "create"), res.log[0]); + // ensure the whitelist is what we expect + let details = query_details(&deps, "foobar".to_string()).unwrap(); + assert_eq!( + details, + DetailsResponse { + id: "foobar".to_string(), + arbiter: HumanAddr::from("arbitrate"), + recipient: HumanAddr::from("recd"), + source: HumanAddr::from("source"), + end_height: None, + end_time: None, + native_balance: vec![], + cw20_balance: vec![Cw20CoinHuman { + address: HumanAddr::from("my-cw20-token"), + amount: Uint128(100) + }], + cw20_whitelist: vec![ + HumanAddr::from("other-token"), + HumanAddr::from("my-cw20-token") + ], + } + ); + // approve it let id = create.id.clone(); let env = mock_env(&deps.api, &create.arbiter, &[]); @@ -508,6 +568,9 @@ mod tests { let res = init(&mut deps, env, init_msg).unwrap(); assert_eq!(0, res.messages.len()); + // only accept these tokens + let whitelist = vec![HumanAddr::from("bar_token"), HumanAddr::from("foo_token")]; + // create an escrow with 2 native tokens let create = CreateMsg { id: "foobar".to_string(), @@ -515,6 +578,7 @@ mod tests { recipient: HumanAddr::from("recd"), end_time: None, end_height: None, + cw20_whitelist: Some(whitelist), }; let sender = HumanAddr::from("source"); let balance = vec![coin(100, "fee"), coin(200, "stake")]; @@ -548,6 +612,26 @@ mod tests { assert_eq!(0, res.messages.len()); assert_eq!(log("action", "top_up"), res.log[0]); + // top with a foreign token not on the whitelist + // top up with one foreign token + let baz_token = HumanAddr::from("baz_token"); + let base = TopUp { + id: create.id.clone(), + }; + let top_up = HandleMsg::Receive(Cw20ReceiveMsg { + sender: HumanAddr::from("random"), + amount: Uint128(7890), + msg: Some(to_binary(&base).unwrap()), + }); + let env = mock_env(&deps.api, &baz_token, &[]); + let res = handle(&mut deps, env, top_up); + match res.unwrap_err() { + StdError::GenericErr { msg, .. } => { + assert_eq!(msg, "Only accepts tokens on the cw20_whitelist") + } + e => panic!("Unexpected error: {}", e), + } + // top up with second foreign token let foo_token = HumanAddr::from("foo_token"); let base = TopUp { diff --git a/contracts/cw20-escrow/src/msg.rs b/contracts/cw20-escrow/src/msg.rs index 9f48ed764..56c56a7e2 100644 --- a/contracts/cw20-escrow/src/msg.rs +++ b/contracts/cw20-escrow/src/msg.rs @@ -1,4 +1,4 @@ -use cosmwasm_std::{Coin, HumanAddr, Uint128}; +use cosmwasm_std::{Api, CanonicalAddr, Coin, HumanAddr, StdResult, Uint128}; use cw20::Cw20ReceiveMsg; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -56,6 +56,19 @@ pub struct CreateMsg { /// block time exceeds this value, the escrow is expired. /// Once an escrow is expired, it can be returned to the original funder (via "refund"). pub end_time: Option, + /// Besides any possible tokens sent with the CreateMsg, this is a list of all cw20 token addresses + /// that are accepted by the escrow during a top-up. This is required to avoid a DoS attack by topping-up + /// with an invalid cw20 contract. See https://github.com/CosmWasm/cosmwasm-plus/issues/19 + pub cw20_whitelist: Option>, +} + +impl CreateMsg { + pub fn canonical_whitelist(&self, api: &A) -> StdResult> { + match self.cw20_whitelist.as_ref() { + Some(v) => v.iter().map(|h| api.canonical_address(h)).collect(), + None => Ok(vec![]), + } + } } pub fn is_valid_name(name: &str) -> bool { @@ -103,6 +116,8 @@ pub struct DetailsResponse { pub native_balance: Vec, /// Balance in cw20 tokens pub cw20_balance: Vec, + /// Whitelisted cw20 tokens + pub cw20_whitelist: Vec, } #[derive(Serialize, Deserialize, Clone, PartialEq, JsonSchema, Debug)] diff --git a/contracts/cw20-escrow/src/state.rs b/contracts/cw20-escrow/src/state.rs index 8646970a5..0b552b7d6 100644 --- a/contracts/cw20-escrow/src/state.rs +++ b/contracts/cw20-escrow/src/state.rs @@ -2,7 +2,8 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use cosmwasm_std::{ - CanonicalAddr, Coin, Env, Order, ReadonlyStorage, StdError, StdResult, Storage, Uint128, + Api, CanonicalAddr, Coin, Env, HumanAddr, Order, ReadonlyStorage, StdError, StdResult, Storage, + Uint128, }; use cosmwasm_storage::{bucket, bucket_read, prefixed_read, Bucket, ReadonlyBucket}; @@ -25,6 +26,8 @@ pub struct Escrow { pub native_balance: Vec, /// Balance in cw20 tokens pub cw20_balance: Vec, + /// All possible contracts that we accept tokens from + pub cw20_whitelist: Vec, } #[derive(Serialize, Deserialize, Clone, PartialEq, JsonSchema, Debug)] @@ -49,6 +52,13 @@ impl Escrow { false } + + pub fn human_whitelist(&self, api: &A) -> StdResult> { + self.cw20_whitelist + .iter() + .map(|h| api.human_address(h)) + .collect() + } } pub const PREFIX_ESCROW: &[u8] = b"escrow";