Submission Details
Severity:
`FjordAuction` contract auction tokens can be drained due to an invalid time check
## Summary
An auction can be ended while the bidding and unbidding is still open for the bidders, which will result in draining the contract auction tokens if a malicious actor calls `auctionEnd()` --> `bid()` --> `claimTokens()` --> `unbid()` when `block.timestamp == auctionEndTime`, where he will receive free auction tokens without participating in bidding.
## Vulnerability Details
-In `FjordAuction` contract: bidders can bid their `fjordPoints` tokens to get auction reward tokens after the auction ends, bidders can ad their bids as long as the auction hasn't ended (`block.timestamp <= auctionEndTime`):
```javascript
function bid(uint256 amount) external {
if (block.timestamp > auctionEndTime) {
revert AuctionAlreadyEnded();
}
bids[msg.sender] = bids[msg.sender].add(amount);
totalBids = totalBids.add(amount);
fjordPoints.transferFrom(msg.sender, address(this), amount);
emit BidAdded(msg.sender, amount);
}
```
- Bidders can remove their bids as long as the auction hasn't ended (`block.timestamp <= auctionEndTime`):
```javascript
function unbid(uint256 amount) external {
if (block.timestamp > auctionEndTime) {
revert AuctionAlreadyEnded();
}
uint256 userBids = bids[msg.sender];
if (userBids == 0) {
revert NoBidsToWithdraw();
}
if (amount > userBids) {
revert InvalidUnbidAmount();
}
bids[msg.sender] = bids[msg.sender].sub(amount);
totalBids = totalBids.sub(amount);
fjordPoints.transfer(msg.sender, amount);
emit BidWithdrawn(msg.sender, amount);
}
```
- So as can be noticed; the auction is supposed to be ended when `block.timestamp > auctionEndTime`, and auction can be ended via calling `FjordAuction.auctionEnd()` that sets the `ended` flag to`true` so that bidders will be able to claim their auction tokens rewards via `FjordAuction.claimTokens()`:
```javascript
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;
}
multiplier = totalTokens.mul(PRECISION_18).div(totalBids);
// Burn the FjordPoints held by the contract
uint256 pointsToBurn = fjordPoints.balanceOf(address(this));
fjordPoints.burn(pointsToBurn);
}
```
```javascript
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);
}
```
- **But** as can be noticed; the auction can be ended when **`block.timestamp == auctionEndTime`** while at the same time allowing bidding and unbidding, and this invalid check can be exploited by any malicious actor as follows:
1. Bidders add their bids, so that the `totalBids` is > 0.
2. Bob is a malicious actor, and at time **`block.timestamp == auctionEndTime`** he makes the following operations in one transaction:
- first : he invokes `auctionEnd()`, so that `ended` flg will be set to `true` allowing auction tokens claim, and the `multiplier` is set based on the current `totalBids`.
- then he calls `bid()` with any amount of points tokens (this is called after calling `auctionEnd()` so that his points token will not be burnt) --> then calls `claimTokens()` where he will be able to receive a claimable rewards tokens proportional to his bid --> then he calls `unbid()` to receive back his bid tokens after receiving his auction rewards.
- So Bob is able to claim auction tokens without participating in auction.
## Impact
So any malicious actor can claim rewards tokens while receiving back their bid tokens as long as the transactions are made when **`block.timestamp == auctionEndTime`**, which will result in exploiting and draining the auction tokens by malicious actors, and leaving the other innocent bidders with a loss as they can't receive rewards if the contract is drained, nor they can redeem their bid tokens as they are burnt when the auction ended.
## Tools Used
Manual Review.
## Recommendations
Update `auctionEnd()`to allow ending the auction when `block.timestamp > auctionEndTime` only:
```diff
function auctionEnd() external {
- if (block.timestamp < auctionEndTime) {
+ if (block.timestamp <= auctionEndTime) {
revert AuctionNotYetEnded();
}
//....
}
```
Updates
Lead Judging Commences
inallhonesty 10 months ago
Submission Judgement Published Assigned finding tags:
Users can bid in the same block when the actionEnd could be called (`block.timestamp==actionEndTime`), depending on the order of txs in block they could lose funds
The protocol doesn't properly treat the `block.timestamp == auctionEndTime` case. Impact: High - There are at least two possible impacts here: 1. By chance, user bids could land in a block after the `auctionEnd()` is called, not including them in the multiplier calculation, leading to a situation where there are insufficient funds to pay everyone's claim; 2. By malice, where someone can use a script to call `auctionEnd()` + `bid(totalBids)` + `claimTokens()`, effectively depriving all good faith bidders from tokens. Likelihood: Low – The chances of getting a `block.timestamp == auctionEndTime` are pretty slim, but it’s definitely possible.
Prize pool breakdown
Total prize
20,000 USDC
nSLOC
66230 USDC / LOC
16,000 USDC
2,500 USDC
1,500 USDC
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.