Avoid using address(this).balance for internal accounting

[code423n4]

Lines of code

https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVault.sol#L466

Vulnerability details

Impact

This could reduce or increase the amount of ETH that token-holders are able to redeem for a reserve token in the case of a buyout success.

Proof of Concept

Using address(this).balance for any internal accounting for smart contract poses risks. Using address(this).balance in a formula to calculate the amount for a user's token redemption is dangerous. Consider the case where it may be advantageous for a malicious attacker to deposit eth into the contract and receive out more than they have deposited. Or consider the case where a malicious actor may deposit into the contract to reduce the amount that another user may be able to redeem. https://consensys.github.io/smart-contract-best-practices/attacks/force-feeding/

https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVault.sol#L466

Tools Used

None

Recommended Mitigation Steps

Avoid using address(this).balance in the internal accounting for the calculation of the redemption.

[GalloDaSballo]

The finding seems to be off of a automated tool and doesn't show how the code can be broken because of using this.balance

[HardlyDifficult]

Yes the balance is used in the redeem function, and someone could selfdestruct to force ETH into that balance. But there's no indication here of how that could be leveraged to damage the accounting - either to extract more than they deposited or to harm the value for others. Without this detail I need to close the report as invalid.