[Lines of code](https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/usd0/modules/USDOOptionsModule.sol#L138-L204) # Vulnerability details ## Impact The [USDOOptionsModule contract]() is a module that is used by the [BaseUSDO contract]() to facilitate functionality for oTap actions. The module functionality is invoked through the invocation of a [delegatecall](https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/usd0/BaseUSDO.sol#L369>) within the BaseUSDO contract's _executeModule function. Additionally, the BaseUSDO contract facilitates the functionality to execute an oTap 'exercise' action on the USDOOptionsModule contract of a [destination block chain](). The function [encodes]() the selector for the USDOOptionsModule contract's [exercise function]() as part of the call data so that the function is invoked once the message has been received by the BaseUSDO contract implementation on the destination block chain. However, the exercise function's [delegatecall]() is problematic because the function invokes a delegatecall on any address that is defined as the [module]() parameter without restriction. Because this is also a public function, a malicious user can pass the address for a contract they have deployed that has functionality with-in it to manipulate the contract's state. For example, this module contract inherits from the [LzApp contract]() which inherits Open Zeppelin's Ownable contract. This means that a contract can be deployed that includes a function with the correct [signature]() with functionality that will update the owner to another address which will then be invoked when the respective contract's address is defined as the module parameter for the exercise function invocation. This would then allow for a malicious user to have complete authority over the module's owner functions. Alternatively, the contract's function implementation can have functionality that will cause selfdestruct to be invoked on the module when the call is delegated to it causing the module's bytecode to be deleted from the address. This would be extremely problematic for the protocol because all calls from the BaseUSDO contract to the respective implementation of the module will fail because there is no way to update the address for the module's implementation after the BaseUSDO contract has been [deployed]() accepts any address as it's module parameter which then has a [call delegated to it]() with the only restriction being that the address must have a function with the [correct signature]() and the call must be [successful](https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/usd0/modules/USDOMarketModule.sol#L168C15-L168C22). A malicious user can use this to then cause functionality to be invoked that will update their address to be the owner of the module, or to invoke selfdestruct on the module which will cause a DOS for the USDO implementation's oTap functionality. ## Tools Used Manual review ## Recommended Mitigation Steps It is recommended to refactor the [USDOOptionsModule contract]() to have functionality that will allow for specific addresses to be approved for the call delegation. This can be done by implementing a state variable mapping with an associated access controlled function to approve which addresses are allowed for the delegatecall: /// @audit recommended mitigation: /// mapping state variable with associated /// function with access control allowing /// module addresses to be approved for call /// delegation mapping(address => bool) public isApproved; function setModuleStatus(address _module, bool _status) external onlyOwner { isApproved[_module] = _status; } Additionally, the [exercise function]() should then be refactored to have an assertion that checks if the defined module parameter has been approved and revert with a custom error if not: function exercise( address module, uint16 _srcChainId, bytes memory _srcAddress, uint64 _nonce, bytes memory _payload ) public { /// @audit recommended mitigation if (!isApproved[module]) revert ModuleNotApproved(); ( , ITapiocaOptionsBrokerCrossChain.IExerciseOptionsData memory optionsData, ITapiocaOptionsBrokerCrossChain.IExerciseLZSendTapData memory tapSendData, ICommonData.IApproval[] memory approvals ) = abi.decode( _payload, ( uint16, ITapiocaOptionsBrokerCrossChain.IExerciseOptionsData, ITapiocaOptionsBrokerCrossChain.IExerciseLZSendTapData, ICommonData.IApproval[] ) ); uint256 balanceBefore = balanceOf(address(this)); bool credited = creditedPackets[_srcChainId][_srcAddress][_nonce]; if (!credited) { _creditTo( _srcChainId, address(this), optionsData.paymentTokenAmount ); creditedPackets[_srcChainId][_srcAddress][_nonce] = true; } uint256 balanceAfter = balanceOf(address(this)); (bool success, bytes memory reason) = module.delegatecall( abi.encodeWithSelector( this.exerciseInternal.selector, optionsData.from, optionsData.oTAPTokenID, optionsData.paymentToken, optionsData.tapAmount, optionsData.target, tapSendData, approvals ) ); if (!success) { if ( balanceAfter - balanceBefore >= optionsData.paymentTokenAmount ) { IERC20(address(this)).safeTransfer( optionsData.from, optionsData.paymentTokenAmount ); } revert(_getRevertMsg(reason)); //forward revert because it's handled by the main executor } emit ReceiveFromChain( _srcChainId, optionsData.from, optionsData.paymentTokenAmount ); } ## Assessed type call/delegatecall --- The text was updated successfully, but these errors were encountered: All reactions