fix issues oak audit no 16 (#417)

contracts/credit-manager/src/stake_astro_lp.rs

@@ -5,7 +5,7 @@ use mars_types::{
 };
 
 use crate::{
-    error::ContractResult,
+    error::{ContractError, ContractResult},
     state::{COIN_BALANCES, INCENTIVES},
     utils::{decrement_coin_balance, increment_coin_balance},
 };
@@ -27,6 +27,10 @@ pub fn stake_lp(deps: DepsMut, account_id: &str, lp_coin: ActionCoin) -> Contrac
         ActionAmount::AccountBalance => coin_balance,
     };
 
+    if new_amount.is_zero() {
+        return Err(ContractError::NoAmount);
+    };
+
     let updated_coin = coin(new_amount.u128(), lp_coin.denom.as_str());
 
     decrement_coin_balance(deps.storage, account_id, &updated_coin)?;

contracts/credit-manager/tests/tests/test_stake_astro_lp.rs

@@ -1,4 +1,5 @@
 use cosmwasm_std::{coins, Addr, Coin, Uint128};
+use mars_credit_manager::error::ContractError;
 use mars_testing::multitest::helpers::AccountToFund;
 use mars_types::credit_manager::{Action, ActionAmount, ActionCoin};
 
@@ -28,6 +29,28 @@ fn stake_claims_rewards() {
         amount: Uint128::new(200),
     };
 
+    // should fail when trying to stake 0 amount
+    let err: ContractError = mock
+        .update_credit_account(
+            &account_id,
+            &user,
+            vec![
+                Action::Deposit(lp_coin.clone()),
+                Action::StakeAstroLp {
+                    lp_token: ActionCoin {
+                        denom: lp_denom.to_string(),
+                        amount: ActionAmount::Exact(Uint128::new(0)),
+                    },
+                },
+            ],
+            &[lp_coin.clone()],
+        )
+        .unwrap_err()
+        .downcast()
+        .unwrap();
+
+    assert_eq!(err, ContractError::NoAmount);
+
     // stake
     mock.update_credit_account(
         &account_id,

contracts/vault/Cargo.toml

@@ -29,6 +29,7 @@ cw-paginate       = { workspace = true }
 cw-vault-standard = { workspace = true }
 mars-owner        = { workspace = true }
 mars-types        = { workspace = true }
+mars-utils        = { workspace = true }
 thiserror         = { workspace = true }
 osmosis-std       = { workspace = true }
 

contracts/vault/src/error.rs

@@ -4,6 +4,7 @@ use cosmwasm_std::{
 };
 use cw_utils::PaymentError;
 use mars_owner::OwnerError;
+use mars_utils::error::ValidationError;
 
 #[derive(Debug, PartialEq, thiserror::Error)]
 pub enum ContractError {
@@ -34,6 +35,9 @@ pub enum ContractError {
     #[error(transparent)]
     Payment(#[from] PaymentError),
 
+    #[error(transparent)]
+    Validation(#[from] ValidationError),
+
     #[error("{0}")]
     Generic(String),
 
@@ -71,6 +75,9 @@ pub enum ContractError {
 
     #[error("Withdrawal interval not passed")]
     WithdrawalIntervalNotPassed {},
+
+    #[error("Invalid cooldown period, expected value greater than 0")]
+    ZeroCooldownPeriod,
 }
 
 pub type ContractResult<T> = Result<T, ContractError>;

contracts/vault/src/instantiate.rs

@@ -1,8 +1,9 @@
 use cosmwasm_std::{DepsMut, Env, MessageInfo, Response};
 use mars_owner::OwnerInit;
+use mars_utils::helpers::validate_native_denom;
 
 use crate::{
-    error::ContractResult,
+    error::{ContractError, ContractResult},
     msg::InstantiateMsg,
     performance_fee::PerformanceFeeState,
     state::{
@@ -42,6 +43,10 @@ pub fn init(
         DESCRIPTION.save(deps.storage, &desc)?;
     }
 
+    if msg.cooldown_period == 0 {
+        return Err(ContractError::ZeroCooldownPeriod {});
+    }
+
     COOLDOWN_PERIOD.save(deps.storage, &msg.cooldown_period)?;
 
     // initialize performance fee state
@@ -53,6 +58,8 @@ pub fn init(
     let vault_token =
         TokenFactoryDenom::new(env.contract.address.to_string(), msg.vault_token_subdenom);
     VAULT_TOKEN.save(deps.storage, &vault_token)?;
+
+    validate_native_denom(&msg.base_token)?;
     BASE_TOKEN.save(deps.storage, &msg.base_token)?;
 
     Ok(vault_token.instantiate()?)

contracts/vault/tests/tests/test_instantiate.rs

@@ -3,6 +3,7 @@ use std::str::FromStr;
 use anyhow::Result as AnyResult;
 use cosmwasm_std::{coin, Addr, Decimal};
 use cw_multi_test::Executor;
+use mars_utils::error::ValidationError;
 use mars_vault::{
     error::ContractError,
     msg::{InstantiateMsg, VaultInfoResponseExt},
@@ -146,6 +147,85 @@ fn cannot_instantiate_with_invalid_performance_fee() {
     );
 }
 
+#[test]
+fn cannot_instantiate_with_zero_cooldown_period() {
+    let fund_manager = Addr::unchecked("fund-manager");
+    let mut mock = MockEnv::new()
+        .fund_account(AccountToFund {
+            addr: fund_manager.clone(),
+            funds: vec![coin(1_000_000_000, "untrn")],
+        })
+        .build()
+        .unwrap();
+    let credit_manager = mock.rover.clone();
+
+    let contract_code_id = mock.app.store_code(mock_managed_vault_contract());
+    let res = mock.app.instantiate_contract(
+        contract_code_id,
+        fund_manager,
+        &InstantiateMsg {
+            base_token: "uusdc".to_string(),
+            vault_token_subdenom: "fund".to_string(),
+            title: None,
+            subtitle: None,
+            description: None,
+            credit_manager: credit_manager.to_string(),
+            cooldown_period: 0,
+            performance_fee_config: PerformanceFeeConfig {
+                fee_rate: Decimal::from_str("0.000046287042457350").unwrap(),
+                withdrawal_interval: 1563,
+            },
+        },
+        &[coin(10_000_000, "untrn")], // Token Factory fee for minting new denom. Configured in the Token Factory module in `mars-testing` package.
+        "mock-managed-vault",
+        None,
+    );
+
+    assert_vault_err(res, ContractError::ZeroCooldownPeriod {});
+}
+
+#[test]
+fn cannot_instantiate_with_invalid_base_denom() {
+    let fund_manager = Addr::unchecked("fund-manager");
+    let mut mock = MockEnv::new()
+        .fund_account(AccountToFund {
+            addr: fund_manager.clone(),
+            funds: vec![coin(1_000_000_000, "untrn")],
+        })
+        .build()
+        .unwrap();
+    let credit_manager = mock.rover.clone();
+
+    let contract_code_id = mock.app.store_code(mock_managed_vault_contract());
+    let res = mock.app.instantiate_contract(
+        contract_code_id,
+        fund_manager,
+        &InstantiateMsg {
+            base_token: "!*jadfaefc".to_string(),
+            vault_token_subdenom: "fund".to_string(),
+            title: None,
+            subtitle: None,
+            description: None,
+            credit_manager: credit_manager.to_string(),
+            cooldown_period: 24,
+            performance_fee_config: PerformanceFeeConfig {
+                fee_rate: Decimal::zero(),
+                withdrawal_interval: 0,
+            },
+        },
+        &[coin(10_000_000, "untrn")], // Token Factory fee for minting new denom. Configured in the Token Factory module in `mars-testing` package.
+        "mock-managed-vault",
+        None,
+    );
+
+    assert_vault_err(
+        res,
+        ContractError::Validation(ValidationError::InvalidDenom {
+            reason: "First character is not ASCII alphabetic".to_string(),
+        }),
+    );
+}
+
 fn assert_vault_err(res: AnyResult<Addr>, err: ContractError) {
     match res {
         Ok(_) => panic!("Result was not an error"),

contracts/vault/tests/tests/test_performance_fee.rs

@@ -124,7 +124,7 @@ fn cannot_withdraw_zero_performance_fee() {
         &mut mock.app,
         &fund_manager,
         &credit_manager,
-        0,
+        1,
         PerformanceFeeConfig {
             fee_rate: Decimal::from_str("0.0000208").unwrap(),
             withdrawal_interval: 60,
@@ -171,7 +171,7 @@ fn cannot_withdraw_if_withdrawal_interval_not_passed() {
         &mut mock.app,
         &fund_manager,
         &credit_manager,
-        0,
+        1,
         PerformanceFeeConfig {
             fee_rate: Decimal::from_str("0.0000208").unwrap(),
             withdrawal_interval: performance_fee_interval,
@@ -277,7 +277,7 @@ fn performance_fee_correctly_accumulated() {
         &mut mock.app,
         &fund_manager,
         &credit_manager,
-        0,
+        1,
         PerformanceFeeConfig {
             fee_rate: Decimal::from_str("0.0000208").unwrap(),
             withdrawal_interval: 60,
@@ -399,6 +399,9 @@ fn performance_fee_correctly_accumulated() {
 
     // -- FOURTH ACTION --
 
+    let unlock_vault_tokens = Uint128::new(10_000_000_000_000);
+    execute_unlock(&mut mock, &user, &managed_vault_addr, unlock_vault_tokens, &[]).unwrap();
+
     // move by 144 hours
     mock.increment_by_time(144 * 60 * 60);
 
@@ -408,8 +411,6 @@ fn performance_fee_correctly_accumulated() {
     let pnl = calculate_pnl(&mut mock, &fund_acc_id, Decimal::from_str("4.9375").unwrap());
     assert_eq!(pnl, Uint128::new(450_000_000));
 
-    let unlock_vault_tokens = Uint128::new(10_000_000_000_000);
-    execute_unlock(&mut mock, &user, &managed_vault_addr, unlock_vault_tokens, &[]).unwrap();
     execute_redeem(
         &mut mock,
         &user,