Ch_301 - gas limit DoS via unbounded operations

[sherlock-admin]

Ch_301

medium

gas limit DoS via unbounded operations

Summary

Only one attack will lead to two types of vulnerabilities in UserManager.sol and UToken.sol

Vulnerability Detail

On UserManager.sol ==> updateTrust()
Case one:
malicious users (members) can keep vouching Alice with trustAmount == 0 until his vouchers array achieves the max limit (2**256-1)
So when a normal member tries to give vouching to Alice with trustAmount != 0 he will find because the vouchers array completely full.

Case two (which is more realistic ):
malicious users (members) can keep vouching Alice with trustAmount == 0 until his vouchers array achieves late’s say 20% of max limit (2**256-1)
The problem is when Alice invoke borrow() or repayBorrow() on UToken.sol

   IUserManager(userManager).updateLocked(msg.sender, uint96(amount + fee), true);
  …
  IUserManager(userManager).updateLocked(borrower, uint96(repayAmount - interest), false);

It will call updateLocked() on UserManager.sol

   function updateLocked(
        address borrower,
        uint96 amount,
        bool lock
    ) external onlyMarket {
        uint96 remaining = amount;

        for (uint256 i = 0; i < vouchers[borrower].length; i++) {
 

The for loop could go through vouchers[] which could be long enough to lead to a "gas limit DoS via unbounded operations"
And the same thing with registerMember(), any user could lose all their fund in this transaction

       function registerMember(address newMember) public virtual whenNotPaused {
        if (stakers[newMember].isMember) revert NoExistingMember();

        uint256 count = 0;
        uint256 vouchersLength = vouchers[newMember].length;

        // Loop through all the vouchers to count how many active vouches there
        // are that are greater than 0. Vouch is the min of stake and trust
        for (uint256 i = 0; i < vouchersLength; i++) {

Impact

1- The user couldn’t get any more vouching
2- The user will be not able to borrow() or repayBorrow()
3- No one can in invokeregisterMember() successfully for a specific user

Code Snippet

   vouchers[borrower].push(Vouch(staker, trustAmount, 0, 0));

https://github.com/sherlock-audit/2022-10-union-finance/blob/main/union-v2-contracts/contracts/user/UserManager.sol#L555

    for (uint256 i = 0; i < vouchers[borrower].length; i++) {

https://github.com/sherlock-audit/2022-10-union-finance/blob/main/union-v2-contracts/contracts/user/UserManager.sol#L807

           uint256 vouchersLength = vouchers[newMember].length;

        // Loop through all the vouchers to count how many active vouches there
        // are that are greater than 0. Vouch is the min of stake and trust
        for (uint256 i = 0; i < vouchersLength; i++) {

https://github.com/sherlock-audit/2022-10-union-finance/blob/main/union-v2-contracts/contracts/user/UserManager.sol#L637-L641

Tool used

Manual Review

Recommendation

Add check for trustAmount == 0

[dmitriia]

Some minimal trustAmount looks to be needed here as the same can be repeated with dust amounts (say 1 wei, as the attacker pays gas anyway, so financially it will not matter).

[GeraldHost]

https://github.com/unioncredit/union-v2-contracts/pull/76/files