TeleportFacet Manual Review Findings

TFT-01M: Inexistent Direct Invocation Protection

Type	Severity	Location
Logical Fault		TeleportFacet.sol:L65-L68

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:

Copy
61/**
62 * @notice sets the state for the TeleportFacet
63 * @dev This method is never attached on the diamond
64 */
65function state(bytes memory data_) external {
66    LibTeleport.Storage storage ts = LibTeleport.teleportStorage();
67    (ts.chainId, ts.providerSelector) = abi.decode(data_, (uint8, address));
68}

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 TeleportFacet::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.

TFT-02M: Contradictory Fee System

Type	Severity	Location
Logical Fault		TeleportFacet.sol:L312, L323

Description:

The TeleportFacet::_transmitWithProvider function will accrue the msg.value transmitted as part of the call as a fee, however, it will forward the value to the providerAddress_. If this address is any other than address(this), the fee accrual system will be permanently corrupted.

Impact:

Should the contract be configured with a providerAddress_ different than the Diamond contract itself, the code will significantly misbehave and increment fees that will never be redeemable corrupting the governance mechanism's removal processes as well as existing rewards.

Example:

Copy
295/**
296 * @notice internal function to transmit the message to the target chain using the given provider
297 * @param targetChainId_ The chainID where the message should be delivered to
298 * @param dappTransmissionReceiver_ The address of the contract in the target chain to receive the transmission
299 * @param dAppId_ ID for the dApp that the message belongs to
300 * @param payload_ The dApp-specific message data
301 * @param providerAddress_ The provider to be used for the transmission
302 * @param extraOptionalArgs_ Extra arguments to be passed to the provider. This allow for specific provider configurations
303 * */
304function _transmitWithProvider(
305    uint8 targetChainId_,
306    bytes calldata dappTransmissionReceiver_,
307    bytes32 dAppId_,
308    bytes calldata payload_,
309    address providerAddress_,
310    bytes memory extraOptionalArgs_
311) private {
312    LibFeeCalculator.accrueReward(msg.value);
313
314    emit TransmissionFees(msg.value);
315
316    bytes memory targetChainTeleportReceiver = LibTeleport
317        .teleportStorage()
318        .teleportAddressByChainId[targetChainId_];
319
320    if (targetChainTeleportReceiver.length == 0) revert TargetChainNotSupported();
321
322    // Call the provider to transmit the message
323    IProvider(providerAddress_).send{ value: msg.value }(
324        targetChainId_,
325        targetChainTeleportReceiver,
326        IProvider.DappTransmissionInfo(
327            _addressToBytes(msg.sender),
328            dappTransmissionReceiver_,
329            dAppId_,
330            payload_
331        ),
332        extraOptionalArgs_
333    );
334}

Recommendation:

We advise the providerAddress_ to be mandated as equal to the address(this) value, or the LibFeeCalculator::accrueReward function to be invoked solely when address(this) == providerAddress_.

We consider either of the two solutions as sufficient in rectifying this issue.

Alleviation (54fd570de24631ca65a7cea022aebe43225a08c7):

The code was updated to properly retain a portion of the msg.value as a fee, forwarding the call to the providerAddress_ safely with a remainingValue amount rather than the full msg.value.

As a result, we consider this exhibit fully alleviated.