code-423n4 / 2023-07-tapioca-findings

12 stars 9 forks source link

Input param `ICommonData.IApproval[] approvals` missing for function `retrieveFromStrategy` #1353

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/Tapioca-DAO/tapiocaz-audit/blob/bcf61f79464cfdc0484aa272f9f6e28d5de36a8f/contracts/tOFT/modules/BaseTOFTStrategyModule.sol#L89-L120

Vulnerability details

Impact

Input param ICommonData.IApproval[] approvals is missing for function retrieveFromStrategy.

Proof of Concept

In other cross chain functions, ICommonData.IApproval[] approvals serves to pass signature to a remote chain. However, in function retrieveFromStrategy, this param is missing. User will need to approve YieldBox for TOFT contract manually on the remote chain.

function retrieveFromStrategy(
    address _from,
    uint256 amount,
    uint256 share,
    uint256 assetId,
    uint16 lzDstChainId,
    address zroPaymentAddress,
    bytes memory airdropAdapterParam
) external payable {

function strategyWithdraw(
    uint16 _srcChainId,
    bytes memory _payload
) public {
    (
        ,
        bytes32 from,
        ,
        uint256 _amount,
        uint256 _share,
        uint256 _assetId,
        address _zroPaymentAddress
    ) = abi.decode(
            _payload,
            (uint16, bytes32, bytes32, uint256, uint256, uint256, address)
        );

    address _from = LzLib.bytes32ToAddress(from);
    _retrieveFromYieldBox(_assetId, _amount, _share, _from, address(this));

The approval is needed at _retrieveFromYieldBox.

Tools Used

Manual

Recommended Mitigation Steps

Add ICommonData.IApproval[] approvals and function _callApproval.

Assessed type

Other

c4-pre-sort commented 1 year ago

minhquanym marked the issue as primary issue

c4-sponsor commented 1 year ago

0xRektora marked the issue as disagree with severity

0xRektora commented 1 year ago

We don't need to use approvals because the intended use is for contracts, not EOAs. But this opens a new problem. Should be marked as Informational.

c4-sponsor commented 1 year ago

0xRektora marked the issue as sponsor confirmed

c4-judge commented 12 months ago

dmvt changed the severity to QA (Quality Assurance)

c4-judge commented 11 months ago

dmvt marked the issue as grade-b