search

code-423n4 / 2021-07-spartan-findings

0 stars 0 forks source link

Strict equality used in claimForMemeber() #143

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

maplesyrup

Vulnerability details

Impact

This is a medium risk vulnerability as an attacker could manipulate the if statement and cause the contract to not work as expected. The if statement controls the final claim to the members LP and zeros out their claim rate. If an attacker manipulates this function, it is possible that they can have a final claim but still have the same claim rate after their final claim.

Proof of Concept

According to Slither Analyzer documentation (https://github.com/crytic/slither/wiki/Detector-Documentation#dangerous-strict-equalities), it is possible that using a strict equality as shown below can be manipulated to behave outside of what its expected. In this case, the if statement controls the state of the claim rate for the member, so if the variable _claimable does not exactly equal mapBondAsset_memberDetails[asset].bondedLP[member] then it will always return false and never zero-out the claim rate when needed possibly allowing an attacker to keep a claim rate even after their final claim.

Slither detection:

BondVault.claimForMember(address,address) (contracts/BondVault.sol, lines#104-117) uses a dangerous strict equality:

_claimable == mapBondAsset_memberDetails[asset].bondedLP[member] (contracts/BondVault.sol, lines#111)

Output results from slither (json format):

"incorrect-equality": [ "BondVault.claimForMember(address,address) (contracts/BondVault.sol#104-117) uses a dangerous strict equality:\n\t- _claimable == mapBondAsset_memberDetails[asset].bondedLP[member] (contracts/BondVault.sol#111)\n", "Pool._addPoolMetrics(uint256) (contracts/Pool.sol#334-347) uses a dangerous strict equality:\n\t- lastMonth == 0 (contracts/Pool.sol#335)\n", "Pool.addForMember(address) (contracts/Pool.sol#173-184) uses a dangerous strict equality:\n\t- baseAmount == 0 || tokenAmount == 0 (contracts/Pool.sol#176)\n", "Pool.addRevenue(uint256) (contracts/Pool.sol#349-355) uses a dangerous strict equality:\n\t- ! (revenueArray.length == 2) (contracts/Pool.sol#350)\n", "Router.addDividend(address,uint256) (contracts/Router.sol#270-282) uses a dangerous strict equality:\n\t- ! (reserve == 0) (contracts/Router.sol#273)\n", "Router.revenueDetails(uint256,address) (contracts/Router.sol#308-320) uses a dangerous strict equality:\n\t- lastMonth == 0 (contracts/Router.sol#309)\n", "SynthVault._addVaultMetrics(uint256) (contracts/synthVault.sol#226-239) uses a dangerous strict equality:\n\t- lastMonth == 0 (contracts/synthVault.sol#227)\n", "SynthVault.addRevenue(uint256) (contracts/synthVault.sol#241-247) uses a dangerous strict equality:\n\t- ! (revenueArray.length == 2) (contracts/synthVault.sol#242)\n" ],

Tools Used

Spartan Contracts Solidity (v 0.8.3) Slither (v 0.8.0)

Recommended Mitigation Steps

  1. Clone repository for Spartan Smart Contracts
  2. Create a python virtual environment with a stable version of python
  3. Install Slither Analyzer on the python VEM
  4. Run Slither against all contracts
ghoul-sol commented 3 years ago

I don't see explicit description of an attack or exploit. Making this best practices hence non-critical