Merge pull request #8 from halotrade-zone/fix-audit-issue

add permission check for execute_create_pair and test case to improve CCOV

contracts/halo-factory/src/contract.rs

@@ -113,18 +113,23 @@ pub fn execute_update_config(
     Ok(Response::new().add_attribute("action", "update_config"))
 }
 
-// Anyone can execute it to create swap pair
+// Only owner of the factory can execute it to create swap pair
 pub fn execute_create_pair(
     deps: DepsMut,
     env: Env,
-    _info: MessageInfo,
+    info: MessageInfo,
     asset_infos: [AssetInfo; 2],
     requirements: CreatePairRequirements,
     commission_rate: Option<Decimal256>,
     lp_token_info: LPTokenInfo,
 ) -> StdResult<Response> {
     let config: Config = CONFIG.load(deps.storage)?;
 
+    // permission check
+    if deps.api.addr_canonicalize(info.sender.as_str())? != config.owner {
+        return Err(StdError::generic_err("unauthorized"));
+    }
+
     // don't allow to create pair with same token
     if asset_infos[0] == asset_infos[1] {
         return Err(StdError::generic_err("same asset"));

contracts/halo-factory/src/tests/test.rs

@@ -288,6 +288,39 @@ fn create_pair() {
         ),
         _ => panic!("DO NOT ENTER HERE"),
     }
+
+    // fail to create pair with invalid factory owner
+    let asset_infos3 = [
+        AssetInfo::NativeToken {
+            denom: "uusd".to_string(),
+        },
+        AssetInfo::Token {
+            contract_addr: "asset0001".to_string(),
+        },
+    ];
+
+    let msg = ExecuteMsg::CreatePair {
+        asset_infos: asset_infos3,
+        requirements: CreatePairRequirements {
+            whitelist: vec![Addr::unchecked("deployer")],
+            first_asset_minimum: Uint128::zero(),
+            second_asset_minimum: Uint128::zero(),
+        },
+        commission_rate: Some(Decimal256::one() + Decimal256::one()),
+        lp_token_info: LPTokenInfo {
+            lp_token_name: "uusd_mAAPL_LP".to_string(),
+            lp_token_symbol: "uusd_mAAPL_LP".to_string(),
+            lp_token_decimals: None,
+        },
+    };
+
+    let env = mock_env();
+    let info = mock_info("addr0001", &[]);
+    let res = execute(deps.as_mut(), env, info, msg).unwrap_err();
+    match res {
+        StdError::GenericErr { msg, .. } => assert_eq!(msg, "unauthorized"),
+        _ => panic!("DO NOT ENTER HERE"),
+    }
 }
 
 #[test]

contracts/halo-router/src/tests/integration_test.rs

@@ -577,6 +577,7 @@ mod tests {
         // Create Pair: NATIVE_DENOM - NATIVE_DENOM_2
         // Update Native Token Decimals is 8 for NATIVE_DENOM and 9 NATIVE_DENOM_2
         // Query Native Token Decimals for NATIVE_DENOM and NATIVE_DENOM_2 on all pairs
+        // for both halo-factory and halo-pair contracts
         // and it should be [8,9] for [NATIVE_DENOM, NATIVE_DENOM_2]
         #[test]
         fn update_native_token_decimals_for_pairs() {

contracts/halo-router/src/tests/test.rs

@@ -951,6 +951,26 @@ fn assert_minimum_receive_token() {
         ),
         _ => panic!("DO NOT ENTER HERE"),
     }
+
+    // assertion failed; unauthorized sender
+    let info = mock_info("addr000", &[]);
+    let msg = ExecuteMsg::AssertMinimumReceive {
+        asset_info: AssetInfo::Token {
+            contract_addr: "token0000".to_string(),
+        },
+        prev_balance: Uint128::zero(),
+        minimum_receive: Uint128::from(1000000u128),
+        receiver: "addr0000".to_string(),
+    };
+
+    let res = execute(deps.as_mut(), mock_env(), info, msg);
+    match res {
+        Err(StdError::GenericErr { msg, .. }) => assert_eq!(
+            msg,
+            "unauthorized: assert_minium_receive only can be called by contract itself"
+        ),
+        _ => panic!("DO NOT ENTER HERE"),
+    }
 }
 
 #[test]