Skip to content

Instantly share code, notes, and snippets.

@danbogd
Last active March 19, 2021 15:51
Show Gist options
  • Save danbogd/43f89d23a8b211c178dd4bfc92147a02 to your computer and use it in GitHub Desktop.
Save danbogd/43f89d23a8b211c178dd4bfc92147a02 to your computer and use it in GitHub Desktop.

Frax-stake Audit Report.

1. Summary

This document is a security audit report performed by danbogd, where Frax-stake have been reviewed.

2. In scope

github commit hash 258285b1783fcc691cb9b1805af3a1ae461c5a22.

3. Findings

In total,7 issues were reported including:

  • 4 low severity issues.
  • 3 notes

No critical security issues were found.

3.1 Used wrong msg sender source

Severity: low/middle

Description

The scope contracts used openzeppelin Ownable contract, derived from Context where it should use _msgSender().

Code snippet

https://github.com/eepdev/frax-stake/blob/258285b1783fcc691cb9b1805af3a1ae461c5a22/Jar_FxsFraxMatic.sol#L12 https://github.com/eepdev/frax-stake/blob/258285b1783fcc691cb9b1805af3a1ae461c5a22/StrategyBase.sol#L9

Recommendation

We suggest to replace msg.sender to _msgSender()

3.2 Known vulnerabilities of ERC-20 token

Severity: low

Description

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.

Recommendation

Add the following code to the transfer() function:

require( recipient != address(this) );

3.3 Transfer is not the last statement

Severity: low/note

Description

There is no bugs now, but it is potentialy dangerous for the re-entrancy as some state change maybe added.

Code snippet

https://github.com/eepdev/frax-stake/blob/258285b1783fcc691cb9b1805af3a1ae461c5a22/StrategyStakingRewardsBase.sol#L72

Recommendation

It is recommended to put transfers to the end of methods.

3.4 Event is probably missing

Severity: low

In method setJar() should probably emit an event.

Code snippet

https://github.com/eepdev/frax-stake/blob/258285b1783fcc691cb9b1805af3a1ae461c5a22/StrategyBase.sol#L70

Recommendation

It is recommended to create new events.

3.5 No validation of the address parameter value in contract constructor

Severity: low

Description

The variable is assigned the value of the constructor input parameter. But this parameter is not checked before this. If the value turns out to be zero, then it will be necessary to redeploy the contract, since there is no other functionality to set this variable.

Code snippet

https://github.com/eepdev/frax-stake/blob/258285b1783fcc691cb9b1805af3a1ae461c5a22/Jar_FxsFraxMatic.sol#L26

Recommendation

It is necessary to add a check of the input parameter to zero before initializing the variables.

3.6 Use constants

Severity: note

Description

In places where you used hardcode addresses you may use constants for save gas.

Code snippet

https://github.com/eepdev/frax-stake/blob/258285b1783fcc691cb9b1805af3a1ae461c5a22/StrategyBase.sol#L31 https://github.com/eepdev/frax-stake/blob/258285b1783fcc691cb9b1805af3a1ae461c5a22/StrategyFxsFrax.sol#L10-L12

3.7 Use the different versions of pragma

Severity: note

Description

Different pragma directives are used: 0.6.7, >= 0.6.0.

Code snippet

https://github.com/eepdev/frax-stake/blob/258285b1783fcc691cb9b1805af3a1ae461c5a22/Jar_FxsFraxMatic.sol#L3 https://github.com/eepdev/frax-stake/blob/258285b1783fcc691cb9b1805af3a1ae461c5a22/StrategyBase.sol#L1 https://github.com/eepdev/frax-stake/blob/258285b1783fcc691cb9b1805af3a1ae461c5a22/StrategyFxsFrax.sol#L2 https://github.com/eepdev/frax-stake/blob/258285b1783fcc691cb9b1805af3a1ae461c5a22/StrategyStakingRewardsBase.sol#L1

Recommendation

Use one Solidity version.

4. Conclusion

The review did not show any critical issues, some low severity issues and notes were found.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment