Auctioneers constructors don't check if teller is of the same type

Summary

All functions constructor does not check if input teller is of the same type as the market. Consequently, someone might deploy for example a Fixed Term market using a Fixed Expiry Teller either by accident or with malicious intent.

The risk here is that the code will not revert and user can just purchase bond like normal but all markets' behaviors are different without the awareness of both market owners and bond buyers

Vulnerability Detail

Let's say for example a Fixed Term FPA market is created using a Fixed Expiry Teller; the result would be all markets become instant swap instead of fixed term bond markets.

The constructor in auctioneer looks likes

constructor(
    IBondTeller teller_,
    IBondAggregator aggregator_,
    address guardian_,
    Authority authority_
) BondBaseFPA(teller_, aggregator_, guardian_, authority_) {}

Since the constructor uses IBondTeller, both Fixed Expiry Teller and Fixed Term Teller could be used to initialize the auctioneer.

After the auctioneer is deployed, Alice creates a fixed term market with vesting = 7 days

If Bob then purchases bond, the function _handlePayout of BondFixedExpiryTeller will be executed, the function is as follow:

    if (vesting_ > uint48(block.timestamp)) {
        expiry = vesting_;
        // Fixed-expiry bonds mint ERC-20 tokens
        bondTokens[underlying_][expiry].mint(recipient_, payout_);
    } else {
        // If no expiry, then transfer payout directly to user
        underlying_.safeTransfer(recipient_, payout_);
    }

Because vesting_ always < block.timestamp in fixed term market; function _handlePayout will always immediately transfer payout token to recipient; effectively become an instant swap market.

Impact

The code will not revert and market behaviors are different without the awareness of the market owners, bond buyers or even the auctioneer deploy

Code Snippet

https://github.com/sherlock-audit/2023-02-bond/blob/main/bonds/src/BondFixedExpiryFPA.sol#L32-#L37 https://github.com/sherlock-audit/2023-02-bond/blob/main/bonds/src/BondFixedExpiryOFDA.sol#L33-#L38 https://github.com/sherlock-audit/2023-02-bond/blob/main/bonds/src/BondFixedExpiryOSDA.sol#L25-#L30 https://github.com/sherlock-audit/2023-02-bond/blob/main/bonds/src/BondFixedTermOFDA.sol#L32-#L37 https://github.com/sherlock-audit/2023-02-bond/blob/main/bonds/src/BondFixedTermOSDA.sol#L24-#L29 https://github.com/sherlock-audit/2023-02-bond/blob/main/bonds/src/BondFixedTermFPA.sol#L31-#L36

Tool used

Manual Review

Recommendation

I recommend checking in each constructor if the teller input is of the same type with the auctioneer being created.

Oighty commented 1 year ago

Acknowledge that this could happen if the teller for an auctioneer is improperly set on construction. However, the tellers do not have an external constant confirming their type so it cannot be checked. Even if the interface is changed to be specific to the vesting type, any address can be supplied. It is up to the deployer to ensure that the auctioneer is properly configured.

hrishibhat commented 1 year ago

It is up to the deployer to ensure that the auctioneer is properly configured.

This is input validation for deployment. Considering this issue as low

sherlock-audit / 2023-02-bond-judging

ktg - Auctioneers constructors don't check if teller is of the same type #40