Platform: Code4rena
Start Date: 09/11/2021
Pot Size: $75,000 USDC
Total HM: 57
Participants: 27
Period: 7 days
Judge: alcueca
Total Solo HM: 49
Id: 52
League: ETH
Rank: 14/27
Findings: 2
Award: $764.62
🌟 Selected for report: 4
🚀 Solo Findings: 0
295.0764 USDC - $295.08
jayjonah8
The whitepaper says that the Vader token contains a Fee-On-Transfer so in XVader.sol, an attacker may be able to keep calling enter() and leave() while being credited more tokens than the contract actually receives eventually draining it.
https://www.financegates.net/2021/07/28/another-polygon-yield-farm-crashes-to-zero-after-exploit/
https://github.com/code-423n4/2021-11-vader/blob/main/contracts/x-vader/XVader.sol
https://www.vaderprotocol.io/whitepaper
Manually code review
There should be pre and post checks on balances to get the real amount
#0 - 0xstormtrooper
2021-11-16T00:41:48Z
Vader fee on transfer will be removed
72.8584 USDC - $72.86
jayjonah8
In the Vader white paper it says that Vader is minted if VETH is burnt 10,000:1, but in the ProtocolConstants.sol it shows 1000:1. Not sure if this is a typo in the whitepaper or if it's a bug.
// Vader -> Vether Conversion Rate (1000:1) uint256 internal constant _VADER_VETHER_CONVERSION_RATE = 1000;
https://github.com/code-423n4/2021-11-vader/blob/main/contracts/shared/ProtocolConstants.sol
Manual code review
If the whitepaper is correct, the _VADER_VETHER_CONVERSION_RATE should be changed to 10000
#0 - SamSteinGG
2021-11-25T11:18:09Z
This is a simple constant that is meant to change prior to launch and does not constitute a high risk vulnerability
#1 - alcueca
2021-12-11T07:15:28Z
Function incorrect as to spec.
🌟 Selected for report: jayjonah8
161.9075 USDC - $161.91
jayjonah8
XVader.sol enter() and leave() functions should have a ReentrancyGuard modifier. It serves as an extra layer of security that can prevent possible future exploits
https://github.com/code-423n4/2021-11-vader/blob/main/contracts/x-vader/XVader.sol
Manual code review
Use Open Zeppelins ReentrancyGuard.sol import and add the modifier to enter() and leave() functions
🌟 Selected for report: jayjonah8
161.9075 USDC - $161.91
jayjonah8
In StakingRewards.sol the _totalSupply variable is manually updated and can be inaccurate if someone sends tokens directly to the contract. The _totalSupply variable is used in withdraw(), stake(), totalSupply(), and rewardPerToken(). This may introduce bugs in future versions as the _totalSupply variable can be different from the actual token balance.
Manual code review
The totalSupply() function on line 53 should return stakingToken.balanceOf(address(this)) and this function should be used everywhere the _totalSupply variable is used.
#0 - 0xstormtrooper
2021-11-15T09:06:19Z
internal _totalSupply is used to protect against shared dilution by a malicious user directly sending token
72.8584 USDC - $72.86
jayjonah8
By using _mint() an NFT can get stuck in a recipients contract if it's not designed to handle them. For this reason ERC721.sol has this warning regarding using _mint() "WARNING: Usage of this method is discouraged, use {_safeMint} whenever possible". This works as an additional guardrail to protect the user from accidentally making a mistake.
Manual code review
Its is recommended to use _safeMint whenever possible.
#0 - SamSteinGG
2021-11-25T11:39:54Z
Duplicate of #27