Currently, while migrating from v1 to v2, we do not check that the migration has not already occured. This allows DAOs to vote to migrate more than once which is a bit of a footgun as migration overrides dao_uri
and activates all proposal modules.
Oak has kindly provided a test case for this <3
#[test]
fn test_replay_migrate() {
// reproduced in contracts/dao-core/src/tests.rs
use cw_core_v1 as v1;
let mut app = App::default();
let govmod_id = app.store_code(sudo_proposal_contract());
let voting_id = app.store_code(cw20_balances_voting());
let core_id = app.store_code(cw_core_contract());
let v1_core_id = app.store_code(v1_cw_core_contract());
let cw20_id = app.store_code(cw20_contract());
let proposal_instantiate = dao_proposal_sudo::msg::InstantiateMsg {
root: CREATOR_ADDR.to_string(),
};
let voting_instantiate = dao_voting_cw20_balance::msg::InstantiateMsg {
token_info: dao_voting_cw20_balance::msg::TokenInfo::New {
code_id: cw20_id,
label: "DAO DAO voting".to_string(),
name: "DAO DAO".to_string(),
symbol: "DAO".to_string(),
decimals: 6,
initial_balances: vec![cw20::Cw20Coin {
address: CREATOR_ADDR.to_string(),
amount: Uint128::from(2u64),
}],
marketing: None,
},
};
const OAK: &str = "oak";
// let oak become the contract admin
let v1_core_instantiate = v1::msg::InstantiateMsg {
admin: Some(OAK.to_string()),
name: "DAO DAO".to_string(),
description: "A DAO that builds DAOs.".to_string(),
image_url: None,
automatically_add_cw20s: false,
automatically_add_cw721s: false,
voting_module_instantiate_info: v1::msg::ModuleInstantiateInfo {
code_id: voting_id,
msg: to_binary(&voting_instantiate).unwrap(),
admin: v1::msg::Admin::CoreContract {},
label: "voting module".to_string(),
},
proposal_modules_instantiate_info: vec![
v1::msg::ModuleInstantiateInfo {
code_id: govmod_id,
msg: to_binary(&proposal_instantiate).unwrap(),
admin: v1::msg::Admin::CoreContract {},
label: "governance module 1".to_string(),
},
v1::msg::ModuleInstantiateInfo {
code_id: govmod_id,
msg: to_binary(&proposal_instantiate).unwrap(),
admin: v1::msg::Admin::CoreContract {},
label: "governance module 2".to_string(),
}, ],
initial_items: None,
};
// init contract with creator as migration admin
let core_addr = app
.instantiate_contract(
v1_core_id,
Addr::unchecked(CREATOR_ADDR),
&v1_core_instantiate,
&[],
"cw-governance",
Some(CREATOR_ADDR.to_string()),
) .unwrap();
// creator performs normal migration
app.execute(
Addr::unchecked(CREATOR_ADDR),
CosmosMsg::Wasm(WasmMsg::Migrate {
contract_addr: core_addr.to_string(),
new_code_id: core_id,
msg: to_binary(&MigrateMsg::FromV1 { dao_uri: None }).unwrap(),
}), )
.unwrap();
// oak disables one proposal module
app.execute_contract(
Addr::unchecked(OAK),
Addr::unchecked(core_addr.to_string()),
&ExecuteMsg::ExecuteAdminMsgs {
msgs: vec![WasmMsg::Execute {
contract_addr: core_addr.to_string(),
msg: to_binary(&ExecuteMsg::UpdateProposalModules {
to_add: vec![],
to_disable: vec!["contract3".to_string()] })
.unwrap(),
funds: vec![],
}
.into()], },
&[],
).unwrap();
// oak pauses the contract
app.execute_contract(
Addr::unchecked(OAK),
Addr::unchecked(core_addr.to_string()),
&ExecuteMsg::ExecuteAdminMsgs {
msgs: vec![WasmMsg::Execute {
contract_addr: core_addr.to_string(),
msg: to_binary(&ExecuteMsg::Pause {
duration: Duration::Height(10),
})
.unwrap(),
funds: vec![],
}
.into()], },
&[],
).unwrap();
// cannot do anything when paused
app.execute_contract(
Addr::unchecked(OAK),
Addr::unchecked(core_addr.to_string()),
&ExecuteMsg::ExecuteAdminMsgs {
msgs: vec![WasmMsg::Execute {
contract_addr: core_addr.to_string(),
msg: to_binary(&ExecuteMsg::UpdateConfig {
DAO".to_string(),
config: Config {
dao_uri: None,
name: "The Empire Strikes Back".to_string(),
description: "haha lol we have pwned your
image_url: None,
automatically_add_cw20s: true,
automatically_add_cw721s: true,
}, })
.unwrap(),
funds: vec![],
}
.into()], },
&[],
).unwrap_err();
// creator replay migrate function to update dao_uri and enable all proposal
modules
let bad = Some("malicious".to_string());
app.execute(
Addr::unchecked(CREATOR_ADDR),
CosmosMsg::Wasm(WasmMsg::Migrate {
contract_addr: core_addr.to_string(),
new_code_id: core_id,
msg: to_binary(&MigrateMsg::FromV1 { dao_uri: bad.clone()
}).unwrap(),
}),
) .unwrap();
// dao_uri is modified in PAUSED state
let dao_uri: DaoURIResponse = app
.wrap()
.query_wasm_smart(core_addr.clone(), &QueryMsg::DaoURI {})
.unwrap();
assert_eq!(dao_uri.dao_uri, bad);
// proposal module can be re-enabled in PAUSED state
let new_state: DumpStateResponse = app
.wrap()
.query_wasm_smart(core_addr.clone(), &QueryMsg::DumpState {})
.unwrap();
let proposal_modules = new_state.proposal_modules;
for (idx, module) in proposal_modules.iter().enumerate() {
let prefix = derive_proposal_module_prefix(idx).unwrap();
assert_eq!(prefix, module.prefix);
assert_eq!(ProposalModuleStatus::Enabled, module.status);
} }
v2-audit