code-423n4 / 2022-01-insure-findings

2 stars 0 forks source link

DOS: `Vault.sol:repayDebt()` allows desync between `totalDebt` and sum of `debts[]` #340

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

Dravee

Vulnerability details

Impact

DOS

Proof of Concept

In Vault.sol:repayDebt(), when a higher amount than the _debt is being sent, the function executes its else statement which can decrease totalDebt by more than it should.

Vault.sol
250:     function repayDebt(uint256 _amount, address _target) external override {
251:         uint256 _debt = debts[_target];
252:         if (_debt >= _amount) {
253:             debts[_target] -= _amount;
254:             totalDebt -= _amount;
255:             IERC20(token).safeTransferFrom(msg.sender, address(this), _amount); 
256:         } else {
257:             debts[_target] = 0;
258:             totalDebt -= _debt;
259:             IERC20(token).safeTransferFrom(msg.sender, address(this), _debt);
260:         }
261:     }

Here, a target is can repay back in excess. While more money seem nice, this might stop future users from repaying their debts as totalDebt -= _debt; could get below 0 and revert the transaction due to an underflow (totalDebt is of type uint256, not int)

Tools Used

VS Code

Recommended Mitigation Steps

Either do not allow for more repayment than the debt, or considerer making totalDebt an int

CloudEllie commented 2 years ago

Withdrawn by warden Dravee: "There's no issue (I must've misread totalDebt -= _amount; instead of totalDebt -= _debt;. Thank you for deleting that submission."