Malicious user can cause denying of batching of unlock requests causing loss to users

[hats-bug-reporter[bot]]

Github username: @aga7hokakological Twitter username: 0xagathokako Submission hash (on-chain): 0x774e345adb4a718e338a882b21b7d163a92433251cc47f00f9934670a39588b6 Severity: high

Description: Description\ In vault contract a malicious user can cause denial for unlocking of tokens by requesting for unlocking of 1 AZERO token.

Attack Scenario\ STEP:

1: All the users stake their tokens in protocol. Say Alice(malicious), Bob, Charlie. Each of them stakes 1M AZERO

2: Then Alice observes first batching and she tries to withdraw some of her AZERO. Charlie and Bob requests for unlocking 0.5M AZERO and Alice requests almost all 999999 AZERO. Keeping 1 AZERO in the protocol.

3: Now when Charlie and Bob requests for unlocking remaining 0.5M AZERO. Alice who has 1 AZERO in protocol also requests for unlocking of tokens.

4: Now there are 2 batches but when the function is called it'll revert.

Attachments

1. Proof of Concept (PoC) File

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

        // Stake 3 million AZERO
        let (_, sess) = helpers::call_stake(sess, &ctx.vault, &ctx.share_token, &ctx.alice, 1_000_000).unwrap();
        let (_, sess) = helpers::call_stake(sess, &ctx.vault, &ctx.share_token, &ctx.bob, 1_000_000).unwrap();
        let (_, sess) = helpers::call_stake(sess, &ctx.vault, &ctx.share_token, &ctx.charlie, 1_000_000).unwrap();

        // Checking how much is nominated
        let (stake1, _unbond, sess) = helpers::query_nominator_balance(sess, &ctx.nominators[0]).unwrap();
        let (stake2, _unbond, sess) = helpers::query_nominator_balance(sess, &ctx.nominators[1]).unwrap();
        assert_eq!(stake1, 1_500_000); // 50% to agent0
        assert_eq!(stake2, 1_500_000); // 50% to agent1

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

        // Request unlocks of all 999_999 + 1_000_000 sAZERO
        let (_, sess) = helpers::call_request_unlock(sess, &ctx.vault, &ctx.share_token, &ctx.alice, 999_999).unwrap();
        let (_, sess) = helpers::call_request_unlock(sess, &ctx.vault, &ctx.share_token, &ctx.charlie, 1_000_000).unwrap();

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

        // Charlie stakes 1M AZERO
        let (_, sess) = helpers::call_stake(sess, &ctx.vault, &ctx.share_token, &ctx.charlie, 1_000_000).unwrap();

        let (total_shares, _, _, sess) = helpers::get_batch_unlock_requests(sess, &ctx.vault, &batch).unwrap();
        // assert_eq!(total_shares, 500_000);

        let (stake1, _unbond, sess) = helpers::query_nominator_balance(sess, &ctx.nominators[0]).unwrap();
        let (stake2, _unbond, sess) = helpers::query_nominator_balance(sess, &ctx.nominators[1]).unwrap();
        assert_eq!(stake1, 2_000_000, "FAILED"); // 50% to agent0
        assert_eq!(stake2, 2_000_000, "FAILED"); // 50% to agent1

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

        // Request unlocks of all 1 million + 1 sAZERO
        let (_, sess) = helpers::call_request_unlock(sess, &ctx.vault, &ctx.share_token, &ctx.bob, 1_000_000).unwrap();
        let (_, sess) = helpers::call_request_unlock(sess, &ctx.vault, &ctx.share_token, &ctx.alice, 1).unwrap();

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

        let (total_pooled_before, sess) = helpers::get_total_pooled(sess, &ctx.vault).unwrap();
        println!("total_pooled_before: {total_pooled_before}");
        let sess = helpers::call_send_batch_unlock_requests(
            sess,
            &ctx.vault,
            &ctx.alice,
            vec![batch, second_batch],
        ).unwrap();

        Ok(())
    }

2. Revised Code File (Optional)

Considering implementing check for minimal withdrawal of AZERO tokens

[coreggon11]

How did alice cause it to be denyed? This poc does not increase time by the time needed for send batch unlock requests to pass, so it is expected to fail

[aga7hokakological]

Yes you're right. I thought why it is failing at first. Didn't come to that conclusion that's why submitted the issue. Yes after increasing the time test passes

[0xmahdirostami]

Thank you for your submission. add let sess = helpers::update_days(sess, 4); before let sess = helpers::call_send_batch_unlock_requests( and the test pass