Malicious inflating of sAZERO supply and spamming of invalid batch request

[hats-bug-reporter[bot]]

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

Description: Description\ A malicious actor could keep inflating sAZERO supply without sending any real value through request_unlock() with a share amount of zero. The effect of this vulnerability on the protocol is users can cheat protocol in order to gain revenue because continous increase of totalSupply of sAZERO will make each shares worth more than the past and spamming of invalid batch request to protocol.

Attack Scenario\ The funtion update_fees() is use to calculates summation of fees from last update until now and can only be called through stake(), send_batch_unlock_requests(), withdraw_fees() and adjust_fee(). withdraw_fees() and adjust_fee() can only be called by a permissioned role while stake() and send_batch_unlock_requests() can be called by the user of this protocol but then the stake() function have a check that prevent users from sending azero that's less than minimum_stake to this function before updating fees. send_batch_unlock_requests() accept only the batch_ids as a parameter which is created by the request_unlock() with a parameter of shares. However, unlike stake() function the request_unlock() doesn't have a check on the amount of shares sent and simply accepts any share amount of any value which implies a zero amount shares can be use by this function to create a batch_id for send_batch_unlock_requests(). A malicious actor can therefore create a batch_id without sending any real value and wait patiently for two days to spam this request to the nomination pool through send_batch_unlock_requests() which will update_fee and inflate the value of sAZERO.

Attachments

1. Proof of Concept (PoC) File

Ade a malicious actor stake 10 Azero and minted 10 sAZERO token. After a while Ade calls request_unlock() for this sAZERO token with a shares value of 0, this function send the share from caller to vault with an amount of zero and then proceed to insert this request to the current batch unlock request which consist of no request to obtains an unlock_id for Ade. After waiting 2 days he can now send_batch_unlock_requests() with batch_ids that was created earlier from the request_unlock() function. This function perform some checks and go ahead to update_fees before sending this value to the nomination pool. After repeating this action for a while, he can go ahead to stake more AZERO inorder to gain more sAZERO than required which he could convert to get away with more AZERO than what's expected by the protocol.

A mitigation to this is to prevent users from sending shares that are less than the minimium amount required or add a require statement that check amount sent is greater zero

2. Revised Code File (Optional)

[bgibers]

Virtual shares represent the revenue entitled to the DAO. They are constantly increasing over time, and are updated within the app when certain functions are called stake(), send_batch_unlock_requests(), withdraw_fees() and adjust_fee() for gas saving purposes.

These shares always exist, and you can see how many currently exist at any given time by calling the function getCurrentVirtualShares(), looking at the implementation, will also give you a better understanding of how they work.

If anything someone who is spamming update_fee is doing the protocol a favor by updating the virtual shares 😄

[skinnyomeje]

Thanks for the response @bgibers although I still believe having no constraint on the amount of shares pass in the _request_unlock()_ can be very much gas ineffective. This I believe should be check because alot of iterations is been perform on batchids in send_batch_unlock_requests()_ and without any real value in these batches, it can cost the protocol more gas to perform this operation. Hench this invalid requests are been sent to the nomination pools which also increases the gas amount unnecessarily

[bgibers]

@skinnyomeje unfortunately gas optimizations are out of scope for this contest 😢