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: 12/27
Findings: 3
Award: $1,816.94
🌟 Selected for report: 8
🚀 Solo Findings: 0
728.5837 USDC - $728.58
hack3r-0m
so if some user wants to vest 1000 tokens, then a malicious actor can vest 1 token on behalf of that user. it will stop user from vesting 1000 tokens.
This can lead to protocol being unusable.
#0 - SamSteinGG
2021-11-25T12:52:41Z
Duplicate of #229
72.8584 USDC - $72.86
hack3r-0m
From https://github.com/vetherasset/vaderprotocol-whitepaper
"VADER is minted if VETH is burnt, 10,000:1 via a Merkle snapshot with 50% upfront and 50% vested over 1 year"
From https://github.com/code-423n4/2021-11-vader/blob/main/contracts/shared/ProtocolConstants.sol#L48
The conversion rate is conflicting
#0 - SamSteinGG
2021-11-25T12:03:48Z
Duplicate #10
🌟 Selected for report: hack3r-0m
161.9075 USDC - $161.91
hack3r-0m
VADER has a maximum supply of 25bn. 7.5bn is claimed by the holders of VETH, and the additional 12.5 bn is paid out as liquidity incentives, 2.5bn for partnerships and 2.5bn for the team linearly vested over 2 years.
https://github.com/code-423n4/2021-11-vader/blob/main/contracts/shared/ProtocolConstants.sol#L19-L29
are conflicting.
(marking it as low risk as per risk estimation guidelines)
🌟 Selected for report: hack3r-0m
161.9075 USDC - $161.91
hack3r-0m
https://github.com/code-423n4/2021-11-vader/blob/main/contracts/tokens/Vader.sol#L122-L131
As per the natspec comment, the Emission event should emit a timestamp of the current era.
#0 - SamSteinGG
2021-11-25T12:10:22Z
As the code indicates, it does. The timestamp of the era is not equivalent to the block.timestamp.
#1 - alcueca
2021-12-11T06:33:57Z
No timestamp is included in the event.
🌟 Selected for report: hack3r-0m
161.9075 USDC - $161.91
hack3r-0m
https://github.com/code-423n4/2021-11-vader/blob/main/contracts/tokens/Vader.sol#L145 says function should be only callable by the deployer.
while in the following scenario:
X is not a deployer but can call setComponents.
Fix the comment or logic to represent the above-mentioned scenario accordingly.
#0 - 0xstormtrooper
2021-11-17T01:06:05Z
We think severity here is 0
#1 - alcueca
2021-12-11T07:19:56Z
I see no grounds to reduce severity. Either the comment is wrong, or the state handling is wrong. Both are severity 1 issues.
#2 - SamSteinGG
2021-12-16T11:22:45Z
@alcueca Can you elaborate what type of risk exists from a comment mistake? There is zero impact to the integrity of the protocol as the code behaves as intended, the "deployer" mentioned does not have any special distinction from any owner of the contract logically.
🌟 Selected for report: hack3r-0m
161.9075 USDC - $161.91
hack3r-0m
https://github.com/code-423n4/2021-11-vader/blob/main/contracts/tokens/Vader.sol#L152
https://github.com/code-423n4/2021-11-vader/blob/main/contracts/tokens/Vader.sol#L178
the owner can pass the owner's same address in address dao and transfer ownership to itself while initializing protocol.
It leads onlyDao modifier restricted functions to be called by the owner.
add check for the new owner is not the current owner.
🌟 Selected for report: hack3r-0m
161.9075 USDC - $161.91
hack3r-0m
vader.sol uses msg.sender at some places while _msgSender() at some, it can cause issue while utilizing meta-transactions.
make it consistent
#0 - SamSteinGG
2021-11-25T12:51:42Z
While this is correct, it is a no risk finding
#1 - alcueca
2021-12-11T07:18:58Z
Using msg.sender and _msgSender has a different behaviour, and might impact state handling. Severity rating is correct.
#2 - SamSteinGG
2021-12-16T11:20:16Z
@alcueca Using msg.sender and _msgSender is equivalent given that the _msgSender implementation of Context retrieves the msg.sender.
🌟 Selected for report: hack3r-0m
68.651 USDC - $68.65
hack3r-0m
https://github.com/code-423n4/2021-11-vader/blob/main/contracts/x-vader/XVader.sol#L5
imports ERC20Votes.sol and inherits from it, but the constructor uses ERC20Permit ( https://github.com/code-423n4/2021-11-vader/blob/main/contracts/x-vader/XVader.sol#L18 )
so import can be changed to ERC20Permit.
#0 - 0xstormtrooper
2021-11-17T01:13:00Z
XVader needs to be ERC20Vote to have voting capabilities.
ERC20Permit has an constructor to set name. ERC20Vote doesnt. We need to call ERC20Permit to initialize name
#1 - alcueca
2021-12-11T05:35:46Z
But you are also calling the ERC20 constructor to set the name. Aren't you setting the name twice? @SamSteinGG?
#2 - SamSteinGG
2021-12-16T11:18:51Z
@alcueca the name utilized in ERC-20 permit is for the EIP-712 signature domain which is distinct from the ERC20 name and symbol.
🌟 Selected for report: hack3r-0m
68.651 USDC - $68.65
hack3r-0m
https://github.com/code-423n4/2021-11-vader/blob/main/contracts/tokens/Vader.sol#L47
lastEmission is not used anywhere in contract logic and can be removed.
🌟 Selected for report: hack3r-0m
68.651 USDC - $68.65
hack3r-0m
https://github.com/code-423n4/2021-11-vader/blob/main/contracts/tokens/USDV.sol#L5
contract USDV is IUSDV, ProtocolConstants, ERC20, Ownable {...)
Ownable is not used anywhere and can be completely removed