DOS/temporary lock of user funds if a validator node is slashed

[hats-bug-reporter[bot]]

Github username: @skinnyomeje Twitter username: SkinneeOmeje Submission hash (on-chain): 0xd46815cc395ec007ee93992e5a82281b340846895c239271590dc163c200bbf6 Severity: high

Description: Description\ Each nomination pool risk staking penalties, with up to 100% of staked funds at risk if validator is malicious. In a case of slashing, send_batch_unlock_requests() will keep on reverting thereby locking the user funds temporarily.

Attachments

1. Proof of Concept (PoC) File

Ade, Bob and Charles staked 400, 300, and 300 AZERO tokens respectively to the vault account. The registry contract which is used for tracking nominator pools and weights consists of three agents. Agents A, B and C and each agent have a weight of 40, 30 and 30 weight distributed among them respectively with a total weight of 100. Each agent delegate funds to nomination pool X, Y, Z which will stake to validator X1, Y1, Z1. Now validator Y1 which is a malicious validator got slashed and all funds allocated to this pool gets confiscated. This slash is been shared proportionally to members actively bonded and thus this lose affects nominator pool Y which in turn affect agent B which formally have a stake amount of 300 AZERO tokens according to the weight allocated but now all gone. After a while, Bob and Charlie decided to unbound by calling request_unlock() function with shares of 300 sAZERO token each (for the purpose of simplicity we keep the ratio 1:1 and exclude fee). This simply transfer the shares to vault, get the current batch id and its total share(i.e 600). Now send_batch_unlock_request() is called after 2 days which is to get the AZERO token from shares which harmlessly return 300 for both B and C respectively. This allocates the unlock quantity across nomination pools by calling the delegate_unbounding(). This function gets the total_weight and agents from registry contract, it proceed to setting the new total pooled to the difference of total pooled and the azero amount(ie 1000 - 600). It also get the weight imbalances which seemingly prioritizes over-allocated pools assuming each bounded stake yielded an increase of 10%. This get_weight_imbalances() function iterate through the agents and query the staked value for each agents which here will get 440 for agent A, 0 for agent B and 330 for agent C. It proceed assign stake_amount_optimal 400 for agent A, 300 for agent B, 300 for Agent C with diff of 40, -30, 30. Here we can see that only agent A and C have a pos_diff with a total of 70. The query staked value is been added to the total_staked which will be 770 and pushed to stakes as [(440), (0), (330)] with an imbalance of [(40), (-300), (30)]. Going back to the delegate_unbounding, phase1 will amount to 70 and phase2 530 with a total_stake_after_phase1 of 700. ln 299 will run a for loop for all agents with a phase1_amount of 40 for A, 0 for B and 30 for C; phase2_amount of 302 for A, 0 for B, 227 for C; and unbound_amount of 342 for A, 0 for B, 257 for C. Summation of this value will leave a dust of 1 token which will be withdrawn from agent A Now this unbound_amount is been withdrawn from agent A and B only with a remaining total amount from both pool to equal 100 token but line 360 set the new total pool to 400 which in this case is a false amount. Although the assume amount that the code say is locked in with the agents is 400 token, the actual amount will be 100 token. If Ade now wishes to unbound his token, his request will definitely fail because the total amount across the whole agents is less than the expected amount. He will therefore only be able to pull out his fund at a mercy of a new stake that is greater than the amount he wish to unbound. Mitigation:

1. First Query all agents for the current funds and a check to know if the total_stake locked across all agent is greater than the total_pooled, hench if less, share this loss equally across all users inorder to avoid one user alone from bearing the whole consequences and the protocol can add additional funds from fee as a form of insurance to minimise damage

2. Avoid staking with pools that delegate fund to a single validator the effect from slashing on a validator can be less effective on agent this way

3. Revised Code File (Optional)

[bgibers]

This is hard to follow/read can you format this @skinnyomeje 😅

This might be a duplicate of #52, but its hard to read through and tell right now

[skinnyomeje]

Sorry for the confusion, this a simplified explanation

1. Ade, Bob and Charles staked 400, 300, 300 AZERO token respectively
2. The registry contract consist of Agent A, B and C with weight of 40:30:30
3. Each agent delegate funds to nomination pool X, Y, Z
4. Nominator Y got slashed and the 300 AZERO allocated to the pool gets confiscated leaving the total staked amount across all agent to be 700
5. Bob and Charles wishes to unbound their AZERO which will result in a total share of 600 token
6. This request is shared across all agent staked in and set the total pooled to 400 although the origin amount left is infact 100 token
7. Ade now wish to unbound his 400 token
8. Although the total pool show an amount of 400 token, He won't be able to unbound his token because what's left in all agent is nothing but 100 AZERO. I hope this is understood

[bgibers]

Duplicate of #51 , and same comment:

"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 🙂"