diff --git a/framework/contracts/account/src/error.rs b/framework/contracts/account/src/error.rs index 95e3808ac..02c37e89d 100644 --- a/framework/contracts/account/src/error.rs +++ b/framework/contracts/account/src/error.rs @@ -106,6 +106,9 @@ pub enum AccountError { #[error("The caller ({caller}) is not the owner account's account ({account}). Only account can create sub-accounts for itself.", )] SubAccountCreatorNotAccount { caller: String, account: String }, + #[error("You can't chain admin calls")] + CantChainAdminCalls {}, + #[error("Abstract Account Address don't match to the Contract address")] AbsAccInvalidAddr { abstract_account: String, diff --git a/framework/contracts/account/src/execution.rs b/framework/contracts/account/src/execution.rs index b92dba82e..cb225ddd5 100644 --- a/framework/contracts/account/src/execution.rs +++ b/framework/contracts/account/src/execution.rs @@ -92,6 +92,9 @@ pub fn admin_execute( ) -> AccountResult { ownership::assert_nested_owner(deps.storage, &deps.querier, &info.sender)?; + if CALLING_TO_AS_ADMIN.exists(deps.storage) { + return Err(AccountError::CantChainAdminCalls {}); + } CALLING_TO_AS_ADMIN.save(deps.storage, &addr)?; let msg = SubMsg::reply_on_success( @@ -184,9 +187,71 @@ mod test { mod execute_action { - use cosmwasm_std::{testing::MOCK_CONTRACT_ADDR, wasm_execute, CosmosMsg}; + use abstract_sdk::namespaces::OWNERSHIP_STORAGE_KEY; + use abstract_std::objects::{gov_type::GovernanceDetails, ownership::Ownership}; + use cosmwasm_std::{ + testing::MOCK_CONTRACT_ADDR, wasm_execute, Addr, Binary, CosmosMsg, DepsMut, Empty, + Env, Response, WasmMsg, + }; + use cw_storage_plus::Item; + + use crate::contract::AccountResult; use super::*; + fn execute_from_res(deps: DepsMut, env: Env, res: Response) -> AccountResult { + // Execute all messages + let info = message_info(&env.contract.address, &[]); + if let CosmosMsg::Wasm(WasmMsg::Execute { + contract_addr: _, + msg, + funds: _, + }) = res.messages[0].msg.clone() + { + execute(deps, env.clone(), info, from_json(&msg)?).map_err(Into::into) + } else { + panic!("Wrong message received"); + } + } + + #[coverage_helper::test] + fn admin_actions_not_chained() -> anyhow::Result<()> { + let mut deps = mock_dependencies(); + deps.querier = abstract_mock_querier(deps.api); + mock_init(&mut deps)?; + let env = mock_env_validated(deps.api); + + // We set the contract as owner. + // We can't make it through execute msgs, because signatures are too messy to reproduce in tests + let ownership = Ownership { + owner: GovernanceDetails::AbstractAccount { + address: env.contract.address.clone(), + } + .verify(deps.as_ref())?, + pending_owner: None, + pending_expiry: None, + }; + + const OWNERSHIP: Item> = Item::new(OWNERSHIP_STORAGE_KEY); + OWNERSHIP.save(deps.as_mut().storage, &ownership)?; + + let msg = ExecuteMsg::AdminExecute { + addr: env.contract.address.to_string(), + msg: to_json_binary(&ExecuteMsg::::AdminExecute { + addr: env.contract.address.to_string(), + msg: Binary::new(vec![]), + })?, + }; + + let info = message_info(&env.contract.address, &[]); + let env = mock_env_validated(deps.api); + + let res = execute(deps.as_mut(), env.clone(), info, msg).unwrap(); + // We simulate it's still an admin call + AUTH_ADMIN.save(deps.as_mut().storage, &true)?; + let res = execute_from_res(deps.as_mut(), env, res); + assert_eq!(res, Err(AccountError::CantChainAdminCalls {})); + Ok(()) + } #[coverage_helper::test] fn only_whitelisted_can_execute() -> anyhow::Result<()> {