sherlock-audit / 2023-12-ubiquity-judging

2 stars 2 forks source link

brgltd - Attacker can manually transfer collateral amount bigger than `poolCeiling` and prevent any new minting #131

Closed sherlock-admin closed 9 months ago

sherlock-admin commented 9 months ago

brgltd

medium

Attacker can manually transfer collateral amount bigger than poolCeiling and prevent any new minting

Summary

freeCollateralBalance will use balanceOf(address(this)) to compute the deposited balance and this value can be manipulated by attackers.

Vulnerability Detail

When minting, there's a check that the freeCollateralBalance is smaller than the pool ceiling. freeCollateralBalance will use balanceOf(address(this)) for checking how much collateral the contract holds. Hence an attacker can transfer the collateral token(s) directly to cause any new minting to revert.

Impact

This can block minting of ubiquity dollar. The attacker could setup a bot that monitors the PoolCeilingSet events and transfers amounts needed to reach the ceiling.

Maybe the attacker could do this during a depeg scenario to further increase instability.

Code Snippet

https://github.com/sherlock-audit/2023-12-ubiquity/blob/main/ubiquity-dollar/packages/contracts/src/dollar/libraries/LibUbiquityPool.sol#L371-L375

https://github.com/sherlock-audit/2023-12-ubiquity/blob/main/ubiquity-dollar/packages/contracts/src/dollar/libraries/LibUbiquityPool.sol#L268-L276

Tool used

Manual review

Recommendation

Store the deposited values in a state variable instead of using balanceOf(address(this)).

If direct transfer of tokens is intended to be supported, you can an an onlyAdmin function that accepts transferred funds to the contract and updates the variable.

Duplicate of #54

sherlock-admin2 commented 9 months ago

1 comment(s) were left on this issue during the judging contest.

auditsea commented:

REF #047

sherlock-admin2 commented 9 months ago

1 comment(s) were left on this issue during the judging contest.

auditsea commented:

REF #047