Omniscia AllianceBlock Audit

FeeCalculatorFacet Manual Review Findings

FeeCalculatorFacet Manual Review Findings

FCF-01M: Inexistent Direct Invocation Protection

Description:

The referenced function is meant to act as a form of state initiation yet does not impose any access control on the logic contract's implementation.

Impact:

While inconsequential in this instance, it is always best practice to not allow the logic contract implementation to be tampered with.

Example:

contracts/facets/FeeCalculatorFacet.sol
13/**
14 * @notice sets the state for the FeeCalculator facet
15 * @param data_ abi encoded data - the service fee for the teleport.
16 * @dev This state method is never attached on the diamond
17 */
18function state(bytes memory data_) external {
19 (LibFeeCalculator.feeCalculatorStorage().serviceFee) = abi.decode(data_, (uint256));
20}

Recommendation:

We advise the function to be solely accessible via delegatecall instructions by storing the address(this) evaluation to an immutable contract variable and ensuring that address(this) != self when the FeeCalculatorFacet::state function is invoked, guaranteeing that the function can only be accessed via delegatecall instructions.

Alleviation (54fd570de24631ca65a7cea022aebe43225a08c7):

Direct call protection was introduced per our recommendation, utilizing a contract-level immutable variable assigned to address(this) during the contract's deployment.