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

2 stars 0 forks source link

Accounting for non-standard ERC20 fees #338

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

Dravee

Vulnerability details

Impact

Bad accounting in case of non-standard ERC20 tokens

Proof of Concept

Vault.sol
105:         IERC20(token).safeTransferFrom(_from, address(this), _amount);
106: 
107:         balance += _amount;

Vault.sol
136:         IERC20(token).safeTransferFrom(_from, address(this), _amount);
137:         balance += _amount;

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:     }

Vault.sol
171:         balance -= _amount;
172:         IERC20(token).safeTransfer(_to, _amount);

Vault.sol
201:     function borrowValue(uint256 _amount, address _to) external onlyMarket override {
202:         debts[msg.sender] += _amount;
203:         totalDebt += _amount;
204: 
205:         IERC20(token).safeTransfer(_to, _amount);
206:     }

Vault.sol
314:         balance -= _retVal;
315:         IERC20(token).safeTransfer(_to, _retVal);

Vault.sol
348:             IERC20(token).safeTransfer(address(controller), _amount);
349:             balance -= _amount;

Tools Used

VS Code

Recommended Mitigation Steps

The balance should be saved before and after the transfer. The difference between those 2 values should then be calculated and added / substracted in the accounting variables.

oishun1112 commented 2 years ago

https://github.com/code-423n4/2022-01-insure-findings/issues/236