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

Flaw in ownership assignment in `FjordAuction`

rhaydden

Summary

The FjordAuction contract when created through the AuctionFactory, inncorrectly assigns ownership to the factory contract address instead of the intended owner (i.e the factory owner). This ownership flaw can cause loss of funds and lack of proper control over individual auctions.

Vulnerability Details

In the FjordAuction contract, the owner is set in the constructor:

constructor(
address _fjordPoints,
address _auctionToken,
uint256 _biddingTime,
uint256 _totalTokens
) {
if (_fjordPoints == address(0)) {
revert InvalidFjordPointsAddress();
}
if (_auctionToken == address(0)) {
revert InvalidAuctionTokenAddress();
}
fjordPoints = ERC20Burnable(_fjordPoints);
auctionToken = IERC20(_auctionToken);
owner = msg.sender; <-------------
auctionEndTime = block.timestamp.add(_biddingTime);
totalTokens = _totalTokens;
}

The AuctionFactory creates new FjordAuction contracts using:

address auctionAddress = address(
new FjordAuction{ salt: salt }(fjordPoints, auctionToken, biddingTime, totalTokens)
);

As a result, msg.sender in the FjordAuction constructor is the AuctionFactory contract address, not the factory owner or the user who initiated the auction creation.

Which means that this incorrect ownership becomes problematic in scenarios such as when no bids are placed:

if (totalBids == 0) {
auctionToken.transfer(owner, totalTokens);
return;
}

In this case, tokens would be transferred to the AuctionFactory contract instead of the intended owner.

Impact

The factory owner loses direct control over individual auctions, as they are not set as the owner. Also in scenarios where funds should be returned to the auction creator (e.g., no bids placed), the funds will instead be sent to the factory contract which may become irretrievable.
Also, aany future functions added to FjordAuction that rely on onlyOwner modifiers would be unusable, as the true owner cannot call them.

Tools Used

Manual review

Recommendations

These are just my personal suggestions that I think will work:

Firstly, consider modifyiing the FjordAuction constructor to accept an explicit owner address:

constructor(
address _fjordPoints,
address _auctionToken,
uint256 _biddingTime,
uint256 _totalTokens,
address _owner
) {
// ... the current implementation ...
owner = _owner;
}

Then update the createAuction function in AuctionFactory to pass the factory owner as the auction owner:

function createAuction(
address auctionToken,
uint256 biddingTime,
uint256 totalTokens,
bytes32 salt
) external onlyOwner {
address auctionAddress = address(
new FjordAuction{ salt: salt }(fjordPoints, auctionToken, biddingTime, totalTokens, owner)
);
// ... the remaining part of the function ...
}
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

If no bids are placed during the auction, the `auctionToken` will be permanently locked within the `AuctionFactory`

An auction with 0 bids will get the `totalTokens` stuck inside the contract. Impact: High - Tokens are forever lost Likelihood - Low - Super small chances of happening, but not impossible

Support

FAQs

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