Frax-stake smart contract security audit report performed by Callisto Security Audit Department
The contracts are vaults for Matic Network/Ethereum that take Quickswap/Uniswap LP token deposits and optimize yields on them.
Website: https://adamant.finance/
Commit 258285b1783fcc691cb9b1805af3a1ae461c5a22
- Jar_FxsFraxMatic.sol
- StrategyBase.sol
- StrategyFxsFrax.sol
- StrategyStakingRewardsBase.sol
The smart contract use open source library from Openzeppelin. Following files was excluded from audit:
- @openzeppelin/contracts/token/ERC20/SafeERC20.sol
- @openzeppelin/contracts/math/SafeMath.sol
- @openzeppelin/contracts/token/ERC20/ERC20.sol
- @openzeppelin/contracts/access/Ownable.sol
The next files were not provided to audit:
- IStrategy.sol
- IJar.sol
- IStakingRewards.sol
- IUniswapV2Pair.sol
- IUniswapRouterV2.sol
In total, 7 issues were reported including:
-
1 medium severity issues.
-
2 low severity issues.
-
3 notes.
-
1 owner privileges.
No critical security issues were found.
Lack of transaction handling mechanism issue. WARNING! This is a very common issue and it already caused millions of dollars losses for lots of token users! More details here.
Add the following code to the transfer(_to address, ...) function:
require( _to != address(this) );
The function withdraw(IERC20 _asset) can be called from a Jar contract only. But a Jar contract has no function which calls it.
The function earn() allow an owner to transfer all tokens from Jar contract to an active staking contract (not often than once per day). Also, the owner has rights to migrate to a new staking contract.
Therefore owner has the ability to steal users tokens using the following steps:
- Call the function earn() at least 1 day before migration.
- Create a fake staking contract that will transfer staked tokens to the owner.
- Migrate to new fake staking contract. After it all users tokens will send beck to
Jarcontract from staking. - Call the function earn() to transfer all users tokens to a fake staking contract.
Users who restake own tokens, may call restake() in 1 day. If you want to allow users to restake only own money you have to replace this code with following :
uint256 lastTimeMigrated = IStrategy(strategy).getLastTimeMigrated();
//Don't allow stakers to call this function if they staked after the migration because that would allow the new staker to move other people's funds
require(lastTimeStaked[msg.sender] < lastTimeMigrated, "Staked after the migration");
//prevent someone to deposit other people's funds
require(lastTimeRestaked[msg.sender] < lastTimeMigrated, "User already restaked");
lastTimeRestaked[msg.sender] = now;There are no bugs now, but it is potentially dangerous for the re-entrance as some state change may be added.
It is recommended to put transfers to the end of methods.
In method setJar() should probably emit an event.
It is recommended to create new events.
In places where you used hardcode addresses, you may use constants to save gas.
https://github.com/eepdev/frax-stake/blob/258285b1783fcc691cb9b1805af3a1ae461c5a22/StrategyBase.sol#L31 https://github.com/eepdev/frax-stake/blob/258285b1783fcc691cb9b1805af3a1ae461c5a22/StrategyFxsFrax.sol#L10-L12
The audited smart contract must not be deployed. Reported issues must be fixed prior to the usage of this contract. Investors have to pay attention to owner's right to transfer there tokens to any contract.
https://gist.github.com/danbogd/43f89d23a8b211c178dd4bfc92147a02