Flash loans can be issued at Zero fees contrary to the Docs that state that loans should revert if the trade is not profitable

[hats-bug-reporter]

Github username: --
Twitter username: --
Submission hash (on-chain): 0xe98ba4f76944c4e43865e558e437fd5de4be99db36c7d64f16928d885ea78b63
Severity: high

Description:
Description
According to the Docs https://dev.spectra.finance/technical-reference/contract-functions/principal-token#flashloan, flash loans should be profitable to the protocol, meaning that Zero fee is not allowed. However, there's currently no check that this requirement is met, allowing borrowers to get loans at Zero fee. Consider the flashLoan function:

function flashLoan(
        IERC3156FlashBorrower _receiver,
        address _token,
        uint256 _amount,
        bytes calldata _data
    ) external override whenNotPaused returns (bool) {
        if (_amount > maxFlashLoan(_token)) revert FlashLoanExceedsMaxAmount();

        uint256 fee = flashFee(_token, _amount);
        _updateFees(fee);

        // Initiate the flash loan by lending the requested IBT amount
        address _ibt = ibt;
        IERC20(_ibt).safeTransfer(address(_receiver), _amount);

        // Execute the flash loan
        if (_receiver.onFlashLoan(msg.sender, _token, _amount, fee, _data) != ON_FLASH_LOAN)
            revert FlashLoanCallbackFailed();

        // Repay the debt + fee
        IERC20(_ibt).safeTransferFrom(address(_receiver), address(this), _amount + fee);

        return true;
    }

the src/tokens/PrincipalToken.sol::flashfee function:

function flashFee(address _token, uint256 _amount) public view override returns (uint256) {
        if (_token != ibt) revert AddressError();
        return _amount._computeFlashloanFee(registry);
    }

and the src/libraries/PrincipalTokenUtil.sol::_computeFlashloanFee function:

 function _computeFlashloanFee(
        uint256 _amount,
        address _registry
    ) internal view returns (uint256) {
        return
            _amount.mulDiv(
                IRegistry(_registry).getPTFlashLoanFee(),
                FEE_DIVISOR,
                Math.Rounding.Ceil
            );
    }

Nowhere do we have a check that the loan is profitable to the protocol.
Attack Scenario
Describe how the vulnerability can be exploited.

Attachments

1. Proof of Concept (PoC) File

For a POC, add this contract at src/mocks/ERC3156FlashBorrowerMock.sol:

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.20;

import {IERC20} from "openzeppelin-contracts/token/ERC20/IERC20.sol";
import {IERC3156FlashBorrower} from "openzeppelin-contracts/interfaces/IERC3156.sol";
import {IERC3156FlashLender} from "openzeppelin-contracts/interfaces/IERC3156.sol";
import {Address} from "openzeppelin-contracts/utils/Address.sol";

/**
 * @dev WARNING: this IERC3156FlashBorrower mock implementation is for testing purposes ONLY.
 * Writing a secure flash lock borrower is not an easy task, and should be done with the utmost care.
 * This is not an example of how it should be done, and no pattern present in this mock should be considered secure.
 * Following best practices, always have your contract properly audited before using them to manipulate important funds on
 * live networks.
 */
contract ERC3156FlashBorrowerMock is IERC3156FlashBorrower {
    bytes32 internal constant _RETURN_VALUE = keccak256("ERC3156FlashBorrower.onFlashLoan");

    bool immutable _enableApprove;
    bool immutable _enableReturn;

    event BalanceOf(address token, address account, uint256 value);
    event TotalSupply(address token, uint256 value);

    constructor(bool enableReturn, bool enableApprove) {
        _enableApprove = enableApprove;
        _enableReturn = enableReturn;
    }

    function onFlashLoan(
        address initiator,
        address token,
        uint256 amount,
        uint256 fee,
        bytes calldata data
    ) public returns (bytes32) {
        //require(address(this) == initiator);

        emit BalanceOf(token, address(this), IERC20(token).balanceOf(address(this)));
        emit TotalSupply(token, IERC20(token).totalSupply());
        
        if (_enableApprove) {
            IERC20(token).approve(msg.sender, amount + fee);
        }

        return _enableReturn ? _RETURN_VALUE : bytes32(0);
    }
}

Also , add this test to test/PrincipalToken/PrincipalToken.t.sol:

function testFlashloan() public {
        uint256 amountToDeposit = 1e18;
        underlying.approve(address(principalToken), amountToDeposit);
        principalToken.deposit(amountToDeposit, MOCK_ADDR_1);
        
        uint256 fee = IERC3156FlashLender(principalToken).flashFee(address(ibt),0.5e18);
        principalToken.flashLoan(IERC3156FlashBorrower(eRC3156FlashBorrowerMock), address(ibt), 0.5e18, bytes(""));
        assertEq(fee,0);
    }

2. Revised Code File (Optional)

• Introduce a check to ensure that loans are profitable to the protocol.

[yanisepfl]

Hello,

We classified this issue as Invalid because:

• Even if the flash fee is set to 0 the protocol never loses funds
• The DAO is responsible for setting the different fees in the protocol, and can decide to set them to 0

Thanks

[0xSwahili]

• "The DAO is responsible for setting the different fees in the protocol, and can decide to set them to 0"- you provided Docs indicating that Zero fees are not allowed. I think that this amounts to changing the contest rules midway.
• Every sensible auditor in this contest would believe that the project is a business venture and would not want to miss making honest profits, unless it's clearly indicated by the project- which is not. Not charging fee without notice is lost profit for any business.

[jeanchambras]

Being profitable to the protocol means that no one can make the vault lose assets while executing a flashloan, not that the fees should never be zero.