From 31e25e4d0c0f0feb7b165e9f38eb962c589d75d4 Mon Sep 17 00:00:00 2001 From: Andrei Zavgorodnii Date: Fri, 22 Mar 2024 15:35:57 +0200 Subject: [PATCH] fix: added proper handling of unsupported message types & tests clippy comments --- .../dao/neutron-chain-manager/src/contract.rs | 5 +- .../src/testing/tests.rs | 47 +++++++++++++++++++ 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/contracts/dao/neutron-chain-manager/src/contract.rs b/contracts/dao/neutron-chain-manager/src/contract.rs index 7b02f50c..55d74f92 100644 --- a/contracts/dao/neutron-chain-manager/src/contract.rs +++ b/contracts/dao/neutron-chain-manager/src/contract.rs @@ -231,9 +231,10 @@ fn check_proposal_execute_message( if typed_proposal.type_field.as_str() == MSG_TYPE_UPDATE_PARAMS_CRON { check_cron_update_msg_params(deps, strategy, proposal)?; + Ok(()) + } else { + Err(ContractError::Unauthorized {}) } - - Ok(()) } /// Checks that the strategy owner is authorised to change the parameters of the /// cron module. We query the current values for each parameter & compare them to diff --git a/contracts/dao/neutron-chain-manager/src/testing/tests.rs b/contracts/dao/neutron-chain-manager/src/testing/tests.rs index 75fec437..8269755d 100644 --- a/contracts/dao/neutron-chain-manager/src/testing/tests.rs +++ b/contracts/dao/neutron-chain-manager/src/testing/tests.rs @@ -334,6 +334,53 @@ pub fn test_execute_execute_message_update_params_cron_authorized() { execute_execute_messages(deps.as_mut(), info.clone(), vec![msg]).unwrap(); } +/// Checks that unsupported message types inside a ProposalExecuteMessage are not +/// executed. +#[test] +pub fn test_execute_execute_message_unsupported_message_type_unauthorized() { + let msg = CosmosMsg::Custom(NeutronMsg::SubmitAdminProposal { + admin_proposal: AdminProposal::ProposalExecuteMessage(ProposalExecuteMessage { + message: r#"{"@type":"/neutron.cron.MsgUnsupported", + "authority":"neutron1hxskfdxpp5hqgtjj6am6nkjefhfzj359x0ar3z", + "params": {"security_address": "addr1", "limit": 16}}"# + .to_string(), + }), + }); + + let mut deps = mock_dependencies(); + let env = mock_env(); + let info = mock_info("neutron_dao_address", &[]); + + instantiate( + deps.as_mut(), + env.clone(), + info.clone(), + InstantiateMsg { + initial_strategy_address: Addr::unchecked("neutron_dao_address".to_string()), + initial_strategy: Strategy::AllowAll, + }, + ) + .unwrap(); + + let info = mock_info("neutron_dao_address", &[]); + execute_add_strategy( + deps.as_mut(), + info.clone(), + Addr::unchecked("addr1".to_string()), + Strategy::AllowOnly(vec![UpdateParamsPermission( + CronUpdateParamsPermissionEnumField(CronUpdateParamsPermission { + security_address: true, + limit: true, + }), + )]), + ) + .unwrap(); + + let info = mock_info("addr1", &[]); + let err = execute_execute_messages(deps.as_mut(), info.clone(), vec![msg]).unwrap_err(); + assert_eq!(err, Unauthorized {}) +} + /// Checks that you can't check the limit if you don't have the permission to do so /// (new style parameter changes). #[test]