`vault.redeem` might revert when the pool is slashed

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0x91abc1bba2532cf2fd11eb61a02f878be41c0284b86cf8e1ac7563a45b707f35 Severity: medium

Description: Description\ Quoting from the comments:

A member is slashed pro-rata based on its stake relative to the total slash amount.

And in vault.reddem, the amount of GasToken(AZERO) to be transferred is based on let azero = share_amount * batch_unlock_request.value_at_redemption.unwrap() / batch_unlock_request.total_shares; defined in vault/lib.rs#L455.

The issue in above formula is that it doesn't consider the case when a pool is slashed.

When a pool is slashed, the vault might less GasToken than deposited, in such case, vault.redeem might revert.

Attack Scenario\ Please consider a simplest case:

1. valut.data.fee_percentage is 0
2. there are two pools, poolId_1, and poolId_2, the weight is 50:50
3. Alice is the only user, she deposits 100e12 AZERO, which means poolId_1 and poolId_2 will receive 50e12 AZERO each.
4. After some time poolId_2 is slashed.
5. Alice calls vault.redeem to retrieve her AZERO.
6. According to vault/lib.rs#L455, the function will 100e12 AZERO, but in fact, the vault has less than 100e12 AZERO, so the function will revert

[bgibers]

Slashing doesn't exist with Aleph Zero, therefore invalid

[0xmahdirostami]

slashing is an inevitable event, if you stake in the nomination pool by your self and do not use Kintsu the risk of slashing remains, i think it will be under other dependencies issues, and there is no issue in our contract. there is no way to handle it, all members in the slashed pool will lose money as well as the Kintsu agent.

[crazy4linux]

Hi @bgibers , I think slashing exists with Aleph Zero according to the alephzero doc here, and quoting from the here:

In case of malicious behavior, coins staked by a validator and his nominators can be slashed.

[crazy4linux]

Supposed the slashing exists in Aleph Zero , I made a POC to prove the slashing will impact the contract. In order to mock the slashing, I changed the mock_nominator/lib.rs as following:

diff --git a/src/mock_nominator/lib.rs b/src/mock_nominator/lib.rs
index cdb3bc9..a6dd993 100644
--- a/src/mock_nominator/lib.rs
+++ b/src/mock_nominator/lib.rs
@@ -85,7 +85,7 @@ mod mock_nominator {
                 return Err(RuntimeError::CallRuntimeFailed);
             } else {
                 if self.unbonded > 0 {
-                    Self::env().transfer(self.vault, self.unbonded)?;
+                    Self::env().transfer(self.vault, self.unbonded * 6 / 10)?;
                     self.unbonded = 0;
                 }

The purpose of the modification is to mock the pool will receive less token when slashing happens. And then please run the following code, in the following code, Alice and Bob stake the same amount of AZERO into the pool, and then the pool is slashed, and then Alice and Bob withdraw their assets back, at the end of the code, bob can't call //let (redeemed, sess) = helpers::call_redeem(sess, &ctx.vault, &ctx.bob, 0).unwrap(); to redeem his token back.

    #[test]
    fn test_staking_slash_redeem() -> Result<(), Box<dyn Error>> {
        let ctx = setup().unwrap();

        // no fee to make the POC easier
        let sess = helpers::call_function(
            ctx.sess,
            &ctx.vault,
            &ctx.bob,
            String::from("adjust_fee"),
            Some(vec![String::from("0")]),
            None,
            helpers::transcoder_vault(),
        )
        .unwrap();

        let (_, _, sess) = helpers::query_nominator_balance(sess, &ctx.nominators[0]).unwrap();
        let (_, _, sess) = helpers::query_nominator_balance(sess, &ctx.nominators[1]).unwrap();

        let (_, sess) = helpers::call_stake(sess, &ctx.vault, &ctx.share_token, &ctx.alice, 1_000_000e10 as u128).unwrap();
        let (_, mut sess) = helpers::call_stake(sess, &ctx.vault, &ctx.share_token, &ctx.bob, 1_000_000e10 as u128).unwrap();

        let (batch, sess) = helpers::query_batch_id(sess, &ctx.vault).unwrap();

        let (_, sess) = helpers::call_request_unlock(sess, &ctx.vault, &ctx.share_token, &ctx.alice, 1_000_000e10 as u128).unwrap();
        let (_, sess) = helpers::call_request_unlock(sess, &ctx.vault, &ctx.share_token, &ctx.bob, 1_000_000e10 as u128).unwrap();

        let (total_shares, _, _, sess) = helpers::get_batch_unlock_requests(sess, &ctx.vault, &batch).unwrap();

        let (staked, unbonded, sess) = helpers::query_nominator_balance(sess, &ctx.nominators[0]).unwrap();
        let (staked, unbonded, sess) = helpers::query_nominator_balance(sess, &ctx.nominators[1]).unwrap();

        let sess = helpers::update_days(sess, 2);

        // Initiate batch unlock request for the first (2 day) batch
        let sess = helpers::call_send_batch_unlock_requests(
            sess,
            &ctx.vault,
            &ctx.bob,
            vec![batch],
        )
        .unwrap();

        let (claimable_fees, mut sess) = helpers::get_current_virtual_shares(sess, &ctx.vault).unwrap();

        let vault_balance = sess.chain_api().balance(&ctx.vault);
        println!("xxx_xxx vault_balance: {}", vault_balance);

        // Wait for cooldown period to complete
        let mut sess = helpers::update_days(sess, 14);

        let alice_balance = sess.chain_api().balance(&ctx.alice);
        println!("xxx_xxx alice_balance: {}", alice_balance);

        // Redeem AZERO plus interest minus fees
        let (redeemed, mut sess) = helpers::call_redeem_with_withdraw(sess, &ctx.vault, &ctx.alice, 0).unwrap();
        println!("xxx_xxx alice redeemed: {}", redeemed);

        // bob can't call redeem here, because the function will revert here
       // if uncommentted the following code, the function will revert
        //let (redeemed, sess) = helpers::call_redeem(sess, &ctx.vault, &ctx.bob, 0).unwrap();
        //println!("xxx_xxx bob   redeemed: {}", redeemed);

        let vault_balance = sess.chain_api().balance(&ctx.vault);
        println!("xxx_xxx vault_balance: {}", vault_balance);

        let alice_balance = sess.chain_api().balance(&ctx.alice);
        println!("xxx_xxx alice_balance: {}", alice_balance);

        Ok(())
    }

[bgibers]

@crazy4linux looking into this a bit deeper, will have an answer for you today

[bgibers]

@crazy4linux currently there is no automatic slashing within AlephZero. We can't impose a solution, when we don't know how slashing will be implemented. We are close with the AZERO team and will address slashing prior to auto slashing coming to fruition 🙂