31507 - [SC - Critical] Malicious user could flash-loan the veALCX to i...
Submitted on May 20th 2024 at 19:48:27 UTC by @savi0ur for Boost | Alchemix
Report ID: #31507
Report type: Smart Contract
Report severity: Critical
Target: https://github.com/alchemix-finance/alchemix-v2-dao/blob/main/src/VotingEscrow.sol
Impacts:
Unintended alteration of what the NFT represents (e.g. token URI, payload, artistic content)
Manipulation of governance voting result deviating from voted outcome and resulting in a direct change from intended effect of original results
Description
Bug Description
https://github.com/alchemix-finance/alchemix-v2-dao/blob/f1007439ad3a32e412468c4c42f62f676822dc1f/src/VotingEscrow.sol#L366-L369
The balanceOfToken()
first checking if ownership change of the _tokenId
is in the current block, if it is then return zero. This check is necessary to have a newly transferred veALCX tokens to have zero voting balance to prevent someone from flash-loaning veALCX to inflate their voting balance.
However, this check is not there in balanceOfTokenAt
and _balanceOfTokenAt
functions.
https://github.com/alchemix-finance/alchemix-v2-dao/blob/f1007439ad3a32e412468c4c42f62f676822dc1f/src/VotingEscrow.sol#L372-L374
https://github.com/alchemix-finance/alchemix-v2-dao/blob/f1007439ad3a32e412468c4c42f62f676822dc1f/src/VotingEscrow.sol#L1426-L1465
As a result, alchemix or some external protocol trying to use balanceOfToken
and balanceOfTokenAt
external functions to find voting balance will return different voting balances for the same _tokenId
depending on which function they called.
https://github.com/alchemix-finance/alchemix-v2-dao/blob/f1007439ad3a32e412468c4c42f62f676822dc1f/src/VotingEscrow.sol#L264-L277
As can be seen, _balanceOfTokenAt
internal function which don't have flashloan protection check is called in getVotes
function to compute voting balance of an account.
Its possible that alchemix or external protocols will use getVotes
function to compute the voting balance of an account to use it in their calculation. Due to the use of _balanceOfTokenAt
function which don't have flashloan protection, will allow users to inflate their voting power by taking a flashloan of veALCX.
https://github.com/alchemix-finance/alchemix-v2-dao/blob/f1007439ad3a32e412468c4c42f62f676822dc1f/src/VotingEscrow.sol#L359-L363
Since, tokenURI
function is also using same vulnerable _balanceOfTokenAt
function, same attack we can perform to change the tokenURI for any _tokenId
.
Impact
Since users are able to inflate their voting power, which they can use to vote for a malicious governance proposal. Same attack we can also use to alter tokenURI.
Recommendation
Flashloan protection check should be implemented in _balanceOfTokenAt
function.
References
https://github.com/alchemix-finance/alchemix-v2-dao/blob/f1007439ad3a32e412468c4c42f62f676822dc1f/src/VotingEscrow.sol#L366-L369
https://github.com/alchemix-finance/alchemix-v2-dao/blob/f1007439ad3a32e412468c4c42f62f676822dc1f/src/VotingEscrow.sol#L372-L374
https://github.com/alchemix-finance/alchemix-v2-dao/blob/f1007439ad3a32e412468c4c42f62f676822dc1f/src/VotingEscrow.sol#L1426-L1465
https://github.com/alchemix-finance/alchemix-v2-dao/blob/f1007439ad3a32e412468c4c42f62f676822dc1f/src/VotingEscrow.sol#L264-L277
https://github.com/alchemix-finance/alchemix-v2-dao/blob/f1007439ad3a32e412468c4c42f62f676822dc1f/src/VotingEscrow.sol#L359-L363
Proof Of Concept
Steps to Run using Foundry:
Paste following foundry code in
src/test/VotingEscrow.t.sol
Run using
FOUNDRY_PROFILE=default forge test --fork-url $FORK_URL --fork-block-number 17133822 --match-contract VotingEscrowTest --match-test testVoteInflationByTransferToken -vvv