Fjord Token Staking
Fjord
DeFiFoundry
20,000 USDC
View results
Previous Next
Submission Details
Severity: high
Invalid

Missing Bid Reset Vulnerability in FjordAuction Contract

lordofterra

Summary

The FjordAuction smart contract contains a critical vulnerability where user bids are not properly reset after claiming tokens. This oversight allows users to potentially claim tokens multiple times, leading to an unfair distribution of auction tokens and potential loss of funds for the contract.

Vulnerability Details

In the claimTokens() function, the contract sets the user's bid to zero after calculating and transferring the claimable tokens:

function claimTokens() external {
if (!ended) {
revert AuctionNotYetEnded();
}
​
uint256 userBids = bids[msg.sender];
if (userBids == 0) {
revert NoTokensToClaim();
}
​
uint256 claimable = userBids.mul(multiplier).div(PRECISION_18);
bids[msg.sender] = 0;
​
auctionToken.transfer(msg.sender, claimable);
emit TokensClaimed(msg.sender, claimable);
}

However, there is no mechanism to prevent a user from calling this function multiple times. The function only checks if the user's bid is zero before proceeding, but does not implement any safeguard against repeated claims.

Impact

1: Users can claim tokens multiple times, receiving more tokens than they are entitled to based on their original bid.

2: This could lead to a depletion of the contract's token balance, potentially leaving later claimants unable to receive their fair share of tokens.

3: The integrity of the auction process is compromised, as the final distribution of tokens does not accurately reflect the bidding results.

4: In extreme cases, a malicious user could drain all available tokens from the contract.

Tools Used

manual review

Recommendations

1: Introduce a new mapping to track whether a user has already claimed their tokens

mapping(address => bool) public hasClaimed;

2: Modify the claimTokens() function to check and update this mapping

function claimTokens() external {
if (!ended) {
revert AuctionNotYetEnded();
}
if (hasClaimed[msg.sender]) {
revert TokensAlreadyClaimed();
}
​
uint256 userBids = bids[msg.sender];
if (userBids == 0) {
revert NoTokensToClaim();
}
​
uint256 claimable = userBids.mul(multiplier).div(PRECISION_18);
bids[msg.sender] = 0;
hasClaimed[msg.sender] = true;
​
auctionToken.transfer(msg.sender, claimable);
emit TokensClaimed(msg.sender, claimable);
}

3: Consider implementing a withdrawal pattern instead of directly transferring tokens, to mitigate potential reentrancy attacks:

mapping(address => uint256) public claimableTokens;
​
function calculateClaims() external {
// ... calculate and store claimable amounts ...
}
​
function withdrawTokens() external {
uint256 amount = claimableTokens[msg.sender];
claimableTokens[msg.sender] = 0;
auctionToken.transfer(msg.sender, amount);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge
over 1 year ago
inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!