Missing SafeMath & SafeCasts

[code423n4]

Handle

cmichel

Vulnerability details

Some calculations don't use SafeMath or do unsafe casts:

Examples:

• ERC20ConvictionScore._updateConvictionScore: bool hasMinimumGovernanceBalance = (int256(balance) + amount) >= governanceMinimumBalance;
• FSDNetwork.openCostShareRequest: user.availableCostShareBenefits - user.openCostShareBenefits
• FSD.mint: getReserveBalance() - bonded
• FSD.mint: int256(bonded)
• FSD.burn: -int256(capitalDesired)

Impact

Not using SafeMath could lead to under-/overflows in certain cases. It also makes it hard to reason about the code, as the reverts only happen at a place that is far from the original overflow/unsafe cast. Updating to a newer Solidity version will break a lot of the code.

Recommendation

Use SafeMath by default. Even if it seems like it's not needed in some places, it could still be that there is some code path that leads to issues that one didn't think of, or gets implemented in the future.

[YunChe404]

58

[pauliax]

While many wardens submitted about potential overflow/underflow issues, none were able to provide a concrete attack path and examples of exploit. Nevertheless, I agree with the approach of better safe than sorry and SafeMath operations should be applied where possible or clearly documented why they are not needed, where there is a 100% certainty it will not cause any harm.

Grouping these issues together and assigning a severity of low. Marking this as a primary issue, as it contains the most general and concise description.