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

Audit Fixes 4: Potential panic during module installation #550

Merged
merged 2 commits into from
Dec 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 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 Down
2 changes: 1 addition & 1 deletion framework/contracts/account/src/modules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ pub fn _install_modules(
add_to_account.push((module.info.id(), module_address.clone()));
install_context.push((module.clone(), Some(module_address)));

Some(init_msg.unwrap())
Some(init_msg.ok_or(AccountError::InitMsgMissing(module.info.id()))?)
}
_ => return Err(AccountError::ModuleNotInstallable(module.info.to_string())),
};
Expand Down
20 changes: 19 additions & 1 deletion framework/contracts/account/tests/apps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use abstract_std::{
registry::ModuleFilter,
};
use abstract_testing::prelude::*;
use cosmwasm_std::{coin, CosmosMsg};
use cosmwasm_std::{coin, CosmosMsg, StdError};
use cw_controllers::{AdminError, AdminResponse};
use cw_orch::prelude::*;

Expand Down Expand Up @@ -81,6 +81,24 @@ fn account_install_app() -> AResult {
Ok(())
}

#[test]
fn account_install_app_without_init_msg() -> AResult {
let chain = MockBech32::new("mock");
let deployment = Abstract::deploy_on(chain.clone(), ())?;
let account = crate::create_default_account(&chain.sender_addr(), &deployment)?;

deployment
.registry
.claim_namespace(account.id()?, "tester".to_owned())?;

let app = MockApp::new_test(chain.clone());
MockApp::deploy(&app, APP_VERSION.parse().unwrap(), DeployStrategy::Try)?;
account
.install_module::<Empty>(APP_ID, None, &[])
.unwrap_err();
Ok(())
}

#[test]
fn account_app_ownership() -> AResult {
let chain = MockBech32::new("mock");
Expand Down
10 changes: 10 additions & 0 deletions framework/packages/abstract-testing/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,16 @@ pub fn abstract_mock_querier_builder(mock_api: MockApi) -> MockQuerierBuilder {
ModuleReference::Account(1),
),
)
.with_contract_map_entry(
// Adding a map module inside the registry
&abstr.registry,
REGISTERED_MODULES,
(
&ModuleInfo::from_id(TEST_MODULE_ID, ModuleVersion::Version(TEST_VERSION.into()))
.unwrap(),
ModuleReference::App(2),
),
)
.with_contract_item(abstr.account.addr(), ACCOUNT_ID, &ABSTRACT_ACCOUNT_ID)
.with_contract_version(abstr.account.addr(), ACCOUNT, TEST_VERSION)
.with_smart_handler(&abstr.module_address, |msg| {
Expand Down
Loading