Summary
While it's understood that the owner is typically a trusted role, this issue could occur due to human error, potentially causing significant problems for users attempting to claim their tokens.
Owner of the FjordAuctionFactory.sol contract is able to create an auction with totalTokens of 0. This will lead to incorect calculation in FjordAuction::endAuction and FjordAuction::claimTokens functions.
Vulnerability Details
If totalTokens is equal to 0 in auction this will lead to incorrect calculation for multiplier variable.
function auctionEnd() external {
if (block.timestamp < auctionEndTime) {
revert AuctionNotYetEnded();
}
if (ended) {
revert AuctionEndAlreadyCalled();
}
ended = true;
emit AuctionEnded(totalBids, totalTokens);
if (totalBids == 0) {
auctionToken.transfer(owner, totalTokens);
return;
}
@audit >>> multiplier = totalTokens.mul(PRECISION_18).div(totalBids);
uint256 pointsToBurn = fjordPoints.balanceOf(address(this));
fjordPoints.burn(pointsToBurn);
}
In case multiplier is 0 this will will lead to miscalculation in claimTokens function. This is simply due to the fact that the claimable variable flows from userBids, multiplied by the multiplier. Thus, if the multiplier is zero, it will always return the claimable amount as zero, regardless of any value for userBids
function claimTokens() external {
if (!ended) {
revert AuctionNotYetEnded();
}
uint256 userBids = bids[msg.sender];
if (userBids == 0) {
revert NoTokensToClaim();
}
@audit >>> uint256 claimable = userBids.mul(multiplier).div(PRECISION_18);
bids[msg.sender] = 0;
auctionToken.transfer(msg.sender, claimable);
emit TokensClaimed(msg.sender, claimable);
}
}
Impact
Users won't be able to claim their tokens since claimable will be equal to 0.
POC
contract AuditTest is Test {
MockERC20 public auctionToken;
MockERC20 public fjordPoints;
FjordAuction public auction;
AuctionFactory factory;
address deployer = makeAddr("deployer");
address owner = makeAddr("owner");
address bidder = makeAddr("bidder");
uint256 totalTokens = 0;
uint256 biddingTime = 3 hours;
bytes32 salt = bytes32(abi.encodePacked("random_value"));
function setUp() public {
auctionToken = new MockERC20("AuctionToekn", "ACT");
fjordPoints = new MockERC20("Fjord", "FJO");
vm.prank(deployer);
factory = new AuctionFactory(address(fjordPoints));
vm.prank(deployer);
factory.createAuction(
address(auctionToken),
biddingTime,
totalTokens,
salt
);
auction = FjordAuction(getAddress(salt));
}
function getAddress(bytes32 _salt) internal view returns (address) {
bytes memory byteData = abi.encodePacked(
type(FjordAuction).creationCode,
abi.encode(fjordPoints, auctionToken, biddingTime, totalTokens)
);
bytes32 hash = keccak256(
abi.encodePacked(
bytes1(0xff),
address(factory),
_salt,
keccak256(byteData)
)
);
return address(uint160(uint256(hash)));
}
function testAuctionWithZeroTotalTokens() public {
uint256 amount = 1e18;
fjordPoints.mint(bidder, amount);
vm.prank(bidder);
fjordPoints.approve(address(auction), type(uint256).max);
vm.prank(bidder);
auction.bid(amount);
vm.warp(4 hours);
vm.prank(deployer);
auction.auctionEnd();
vm.prank(bidder);
auction.claimTokens();
assert(auctionToken.balanceOf(bidder) == 0);
}
}
Tools Used
Recommendations
Add check to ensure that when auction is created totalTokens are greather than 0
Example
function createAuction(
address auctionToken,
uint256 biddingTime,
uint256 totalTokens,
bytes32 salt
) external onlyOwner {
+ require(totalTokens > 0, "totalTokens must be greather than 0")
address auctionAddress = address(
new FjordAuction{ salt: salt }(fjordPoints, auctionToken, biddingTime, totalTokens)
);
// Transfer the auction tokens from the msg.sender to the new auction contract
IERC20(auctionToken).transferFrom(msg.sender, auctionAddress, totalTokens);
emit AuctionCreated(auctionAddress);
}
