Skip to content

Commit

Permalink
Audit fixes (#558)
Browse files Browse the repository at this point in the history
Co-authored-by: Interchain Adair <[email protected]>
  • Loading branch information
Kayanski and adairrr authored Dec 11, 2024
1 parent d7e7e04 commit bf18c4e
Show file tree
Hide file tree
Showing 30 changed files with 922 additions and 228 deletions.
78 changes: 75 additions & 3 deletions framework/Cargo.lock

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

7 changes: 6 additions & 1 deletion framework/contracts/account/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ crate-type = ["cdylib", "rlib"]
[features]
default = ["export"]
export = []
xion = ["abstract-xion"]
xion = ["abstract-xion", "abstract-std/xion"]

[package.metadata.optimizer]
builds = [{ name = "xion", features = ["xion"] }]
Expand Down Expand Up @@ -74,5 +74,10 @@ rstest = { workspace = true }

base64 = { version = "0.22.1", default-features = false }

## For xion tests, with signatures
serial_test = "3.2.0"
xionrs = { version = "0.19.0-pre", package = "cosmrs", git = "https://github.com/CyberHoward/cosmos-rust.git", branch = "patch-1" }


[profile.release]
overflow-checks = true
42 changes: 26 additions & 16 deletions framework/contracts/account/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,7 @@ use abstract_sdk::{
};
use abstract_std::{
account::{
state::{
AccountInfo, WhitelistedModules, AUTH_ADMIN, INFO, SUSPENSION_STATUS,
WHITELISTED_MODULES,
},
state::{AccountInfo, WhitelistedModules, INFO, SUSPENSION_STATUS, WHITELISTED_MODULES},
UpdateSubAccountAction,
},
module_factory::SimulateInstallModulesResponse,
Expand Down Expand Up @@ -150,15 +147,15 @@ pub fn instantiate(
ensure_eq!(
address,
env.contract.address,
AccountError::AbsAccInvalidAddr {
AccountError::AbstractAccountInvalidAddress {
abstract_account: address.to_string(),
contract: env.contract.address.to_string()
}
);
#[cfg(feature = "xion")]
{
let Some(mut add_auth) = authenticator else {
return Err(AccountError::AbsAccNoAuth {});
return Err(AccountError::AbstractAccountNoAuth {});
};
abstract_xion::execute::add_auth_method(deps.branch(), &env, &mut add_auth)?;

Expand All @@ -172,7 +169,7 @@ pub fn instantiate(
}
// No Auth possible - error
#[cfg(not(feature = "xion"))]
return Err(AccountError::AbsAccNoAuth {});
return Err(AccountError::AbstractAccountNoAuth {});
}
_ => (),
};
Expand Down Expand Up @@ -269,24 +266,24 @@ pub fn execute(mut deps: DepsMut, env: Env, info: MessageInfo, msg: ExecuteMsg)

match msg {
// ## Execution ##
ExecuteMsg::Execute { msgs } => execute_msgs(deps, &info.sender, msgs),
ExecuteMsg::Execute { msgs } => execute_msgs(deps, env, &info.sender, msgs),
ExecuteMsg::AdminExecute { addr, msg } => {
let addr = deps.api.addr_validate(&addr)?;
admin_execute(deps, info, addr, msg)
}
ExecuteMsg::ExecuteWithData { msg } => {
execute_msgs_with_data(deps, &info.sender, msg)
execute_msgs_with_data(deps, env, &info.sender, msg)
}
ExecuteMsg::ExecuteOnModule {
module_id,
exec_msg,
funds,
} => execute_on_module(deps, info, module_id, exec_msg, funds),
} => execute_on_module(deps, env, info, module_id, exec_msg, funds),
ExecuteMsg::AdminExecuteOnModule { module_id, msg } => {
admin_execute_on_module(deps, info, module_id, msg)
}
ExecuteMsg::IcaAction { action_query_msg } => {
ica_action(deps, info, action_query_msg)
ica_action(deps, env, info, action_query_msg)
}

// ## Configuration ##
Expand Down Expand Up @@ -354,14 +351,15 @@ pub fn execute(mut deps: DepsMut, env: Env, info: MessageInfo, msg: ExecuteMsg)
unreachable!("Update status case is reached above")
}
ExecuteMsg::AddAuthMethod { add_authenticator } => {
add_auth_method(deps, env, add_authenticator)
add_auth_method(deps, env, info, add_authenticator)
}
#[allow(unused)]
ExecuteMsg::RemoveAuthMethod { id } => remove_auth_method(deps, env, id),
ExecuteMsg::RemoveAuthMethod { id } => remove_auth_method(deps, env, info, id),
}
}
}?;
AUTH_ADMIN.remove(deps.storage);

#[cfg(feature = "xion")]
abstract_std::account::state::AUTH_ADMIN.remove(deps.storage);
Ok(response)
}

Expand Down Expand Up @@ -425,10 +423,22 @@ pub fn sudo(
msg: abstract_xion::contract::AccountSudoMsg,
) -> abstract_xion::error::ContractResult<Response> {
if let abstract_xion::contract::AccountSudoMsg::BeforeTx { .. } = &msg {
AUTH_ADMIN.save(deps.storage, &true)?;
abstract_std::account::state::AUTH_ADMIN.save(deps.storage, &true)?;
};
if let abstract_xion::contract::AccountSudoMsg::AfterTx { .. } = &msg {
abstract_std::account::state::AUTH_ADMIN.remove(deps.storage);
};
abstract_xion::contract::sudo(deps, env, msg)
}
#[cfg(not(feature = "xion"))]
#[cfg_attr(feature = "export", cosmwasm_std::entry_point)]
pub fn sudo(
_deps: DepsMut,
_env: Env,
_msg: cosmwasm_std::Empty,
) -> Result<cosmwasm_std::Response, AccountError> {
unimplemented!()
}

/// Verifies that *sender* is the owner of *nft_id* of contract *nft_addr*
fn verify_nft_ownership(
Expand Down
24 changes: 18 additions & 6 deletions framework/contracts/account/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ pub enum AccountError {
#[error("The provided module {0} can't be installed on an Abstract account")]
ModuleNotInstallable(String),

#[error("The module {0} needs an init msg to be installed but not was provided")]
InitMsgMissing(String),

#[error("The name of the proposed module can not have length 0.")]
InvalidModuleName {},

Expand All @@ -51,7 +54,7 @@ pub enum AccountError {
#[error("Cannot migrate {} twice", module_id)]
DuplicateModuleMigration { module_id: String },

#[error("{0} not upgradable")]
#[error("{0} not upgradeable")]
NotUpgradeable(ModuleInfo),

#[error("Cannot remove module because {0:?} depend(s) on it.")]
Expand All @@ -78,8 +81,11 @@ pub enum AccountError {
NotWhitelisted {},

// ** Sub Account ** //
#[error("Removing sub account failed")]
SubAccountRemovalFailed {},
#[error("Sub account doesn't exist")]
SubAccountDoesntExist {},

#[error("Sub account is expected to be the caller")]
SubAccountIsNotCaller {},

#[error("Register of sub account failed")]
SubAccountRegisterFailed {},
Expand All @@ -106,14 +112,20 @@ 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("Abstract Account Address don't match to the Contract address")]
AbsAccInvalidAddr {
#[error("You can't chain admin calls")]
CantChainAdminCalls {},

#[error("Abstract Account Address ({abstract_account}) doesn't match to the Contract address ({contract}) ")]
AbstractAccountInvalidAddress {
abstract_account: String,
contract: String,
},

#[error("No auth methods capabilities on this account (xion feature disabled)")]
NoAuthFeature {},

#[error("Abstract Account don't have Authentication")]
AbsAccNoAuth {},
AbstractAccountNoAuth {},

#[cfg(feature = "xion")]
#[error(transparent)]
Expand Down
Loading

0 comments on commit bf18c4e

Please sign in to comment.