No checks for minimum_stake in request_unlock lead can lead to bloating of storage

[hats-bug-reporter[bot]]

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

Description: Description\ When staking tokens in the vault contract in stake, a minimum_stake is enforced so that users cannot bloat the storage with low-value positions. Unfortunately there is no check when calling request_unlock. This leads to users being able to call this function with shares=1 which adds an entry to data.batch_unlock_requests.

Attack Scenario\ Assuming the current price of AZERO of about 0.8 USD and the 10 decimals used in the other tests, an attacker could once call stake with 100e10 AZERO, opening one position worth 80 USD. They can then call request_unlock 100e10 times with a value of 1. This will add 100e10 elements to data.batch_unlock_requests. When testing, 515 unlock requests already bloated the storage to a point where no new unlock requests could be added and all further calls would revert.

Attachments

1. Proof of Concept (PoC) File

   Add the following to drink_tests/lib.rs:

   #[test]
   fn test_storage_bloat() {
       let ctx = setup().unwrap();
   
       // Verify nominators
       let (staked, unbonded, sess) = helpers::query_nominator_balance(ctx.sess, &ctx.nominators[0]).unwrap();
       assert_eq!(staked, 0, "Nominator #1 should have no staked AZERO");
       assert_eq!(unbonded, 0, "Nominator #1 should have no unbonded AZERO");
       let (staked, unbonded, sess) = helpers::query_nominator_balance(sess, &ctx.nominators[1]).unwrap();
       assert_eq!(staked, 0, "Nominator #2 should have no staked AZERO");
       assert_eq!(unbonded, 0, "Nominator #2 should have no unbonded AZERO");
   
       // Staking of 100 AZERO worth ~80 USD
       let (_, sess) = helpers::call_stake(sess, &ctx.vault, &ctx.share_token, &ctx.alice, 100e10 as u128).unwrap();
   
       let mut iter = 0;
       let mut sess_ = sess;
       while iter < 515 as u128 {
           (_, sess_) = helpers::call_request_unlock(sess_, &ctx.vault, &ctx.share_token, &ctx.alice, 1).unwrap();
           iter += 1;
       }
   }

2. Revised Code File (Optional)

Consider also adding a minimum size for request_unlock:

 diff1.git
diff --git a/src/vault/lib.rs b/src/vault/lib.rs
index 3ec4acb..3cfab31 100755
--- a/src/vault/lib.rs
+++ b/src/vault/lib.rs
@@ -251,6 +251,10 @@ mod vault {
             let caller = Self::env().caller();
             let now = Self::env().block_timestamp();

+            if shares < self.data.minimum_stake {
+                return Err(VaultError::MinimumStake);
+            }
+
             self.transfer_shares_from(&caller, &Self::env().account_id(), shares)?;

             let current_batch_unlock_id = self.data.get_batch_unlock_id(now);

[coreggon11]

The user can only make ~500 calls then they would run out of storage for themselves. This will break the funcitonality for them by wrong usage.

[0xmahdirostami]

Thank you for your submission. batch_unlock_requests is mapping. users could add the item to user_unlock_requests, which will DOS himself