Summary
The FjordAuction contract's constructor is missing a zero-value check for the _totalTokens parameter, which may lead to bidding Fjord points in exchange for zero auction tokens.
Vulnerability Details
I understand that the owners are trusted and the chance of misconfiguration is very low. However, if there is a zero address check for the fjordPoints and auctionToken parameters in FjordAuction:constructor(), the arguments about trust and misconfiguration are not entirely valid, and the same check should apply to _totalTokens.
There is no setter method for totalTokens. If totalTokens is set to zero, the multiplier storage variable, which is used to calculate the amount of auction tokens claimable per FjordPoint bid, will also be zero.
FjordAuction::auctionEnd()
multiplier = totalTokens.mul(PRECISION_18).div(totalBids);
Then, if a user bids Fjord points and calls claimTokens() after the auction ends, the claimable tokens will be zero. The auctionEnd() function burns all Fjord points, meaning the user loses their Fjord points and does not receive any auction tokens.
FjordAuction::claimTokens()
uint256 claimable = userBids.mul(multiplier).div(PRECISION_18);
FjordAuction::auctionEnd()
uint256 pointsToBurn = fjordPoints.balanceOf(address(this));
fjordPoints.burn(pointsToBurn);
Impact
The user loses their bidded Fjord points and receives zero auction tokens.
PoC
Import the console.sol in test/unit/auction.t.sol.
import "forge-std/console.sol";
Add the following test case in test/unit/auction.t.sol.
function testZeroClaimTokensWhenTotalTokensIsZero() public {
FjordAuction newAuction =
new FjordAuction(address(fjordPoints), address(auctionToken), biddingTime, 0);
address bidder = address(0x2);
uint256 bidAmount = 100 ether;
deal(address(fjordPoints), bidder, bidAmount, true);
console.log("bidder balance of fjordPoints before bid", fjordPoints.balanceOf(bidder));
assertEq(fjordPoints.balanceOf(bidder), 100 ether);
vm.startPrank(bidder);
fjordPoints.approve(address(newAuction), bidAmount);
newAuction.bid(bidAmount);
vm.stopPrank();
console.log("bidder balance of fjordPoints after bid", fjordPoints.balanceOf(bidder));
assertEq(fjordPoints.balanceOf(bidder), 0);
console.log("auction balance of fjordPoints after bid", fjordPoints.balanceOf(address(newAuction)));
assertEq(fjordPoints.balanceOf(address(newAuction)), 100 ether);
skip(biddingTime);
newAuction.auctionEnd();
console.log("auction balance of fjordPoints after auction end", fjordPoints.balanceOf(address(newAuction)));
assertEq(fjordPoints.balanceOf(address(newAuction)), 0);
vm.prank(bidder);
newAuction.claimTokens();
uint256 expectedTokens = 0;
console.log("bidder balance of auctionToken after token claim", auctionToken.balanceOf(bidder));
assertEq(auctionToken.balanceOf(bidder), expectedTokens);
}
Execute it with forge test -vv --mt testZeroClaimTokensWhenTotalTokensIsZero and the result should be similar to the following output:
Ran 1 test for test/unit/auction.t.sol:TestAuction
[PASS] testZeroClaimTokensWhenTotalTokensIsZero() (gas: 1057986)
Logs:
bidder balance of fjordPoints before bid 100000000000000000000
bidder balance of fjordPoints after bid 0
auction balance of fjordPoints after bid 100000000000000000000
auction balance of fjordPoints after auction end 0
bidder balance of auctionToken after token claim 0
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 4.99ms (2.35ms CPU time)
Ran 1 test suite in 7.63ms (4.99ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Tools Used
Manual, Foundry
Recommendations
Add zero value check for _totalTokens in FjordAuction:constructor()
@@ -23,6 +23,11 @@ contract FjordAuction {
*/
error InvalidAuctionTokenAddress();
+ /**
+ * @notice Thrown when the provided total tokens value is invalid (zero).
+ */
+ error InvalidTotalTokensValue();
+
/**
* @notice Thrown when a bid is placed after the auction has already ended.
*/
@@ -133,6 +138,10 @@ contract FjordAuction {
auctionToken = IERC20(_auctionToken);
owner = msg.sender;
auctionEndTime = block.timestamp.add(_biddingTime);
+
+ if (_totalTokens == 0) {
+ revert InvalidTotalTokensValue();
+ }
totalTokens = _totalTokens;
}
