-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Governance relies on current totalSupply of bHermes when calculate proposalThresholdAmount
and quorumVotesAmount
#179
Comments
trust1995 marked the issue as primary issue |
trust1995 marked the issue as satisfactory |
0xLightt marked the issue as sponsor confirmed |
trust1995 marked issue #180 as primary and marked this issue as a duplicate of 180 |
I believe this is valid as it is something we want to address (save the totalSupply at the time of the creation of every proposal) and it is not a duplicate of #180. |
trust1995 marked the issue as not a duplicate |
trust1995 marked the issue as selected for report |
Lines of code
https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/governance/GovernorBravoDelegateMaia.sol#L87-L93
Vulnerability details
Impact
As people mint bHermes, bHermesVotes' totalSupply grows. And
quorumVotesAmount
to execute proposal also grows. But it shouldn't, because new people can't vote for it. This behavior adds inconsistency to voting process, because changes threshold after creating proposal.Proof of Concept
Here you can see that Governance fetches current totalSupply:
https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/governance/GovernorBravoDelegateMaia.sol#L87-L93
bHermes is ERC4626DepositOnly and mints new govToken when user calls
deposit()
ormint()
, thus increasing totalSupply:https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/hermes/bHermes.sol#L123-L133
Tools Used
Manual Review
Recommended Mitigation Steps
Add parameter
totalSupply
to Proposal struct and use it instead of current totalSupply in functionsgetProposalThresholdAmount()
andgetQuorumVotesAmount()
Assessed type
Governance
The text was updated successfully, but these errors were encountered: