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

15 stars 10 forks source link

Cross-Chain Signature Replay Attack #763

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/MarketERC20.sol#L251-L284 https://github.com/Tapioca-DAO/YieldBox/blob/f5ad271b2dcab8b643b7cf622c2d6a128e109999/contracts/YieldBoxPermit.sol#L72-L90 https://github.com/Tapioca-DAO/YieldBox/blob/f5ad271b2dcab8b643b7cf622c2d6a128e109999/contracts/YieldBoxPermit.sol#L42-L61

Vulnerability details

Impact

User operations can be replayed on smart account across different chains. This can lead to users losing funds or any unexpected behavior that transaction replay attacks usually lead to. This will allow attacker to unauthorized approve tokens. Attacker can perform admin operations

Proof of Concept

As specified by the EIP4337 standard to prevent replay attacks ... the signature should depend on chainid. In MarketERC20.sol#_permit & YieldBoxPermit.sol#_permit the chainId is missing which means that the same UserOperation can be replayed on a different chain for the same smart contract account.

Tools Used

Manual review

Recommended Mitigation Steps

Add the chainId in the calculation of the UserOperation hash in MarketERC20.sol#_permit & YieldBoxPermit.sol#_permit

  function _permit(
        bool asset, // 1 = asset, 0 = collateral
        address owner,
        address spender,
        uint256 value,
        uint256 deadline,
        uint8 v,
        bytes32 r,
        bytes32 s
    ) internal {
        require(block.timestamp <= deadline, "ERC20Permit: expired deadline");

        bytes32 structHash = keccak256(
            abi.encode(
                asset ? _PERMIT_TYPEHASH : _PERMIT_TYPEHASH_BORROW,
                owner,
                spender,
                value,
                _useNonce(owner),
                deadline
            )
        );

        bytes32 hash = _hashTypedDataV4(structHash);

        address signer = ECDSA.recover(hash, v, r, s);
        require(signer == owner, "ERC20Permit: invalid signature");

        if (asset) {
            _approve(owner, spender, value);
        } else {
            _approveBorrow(owner, spender, value);
        }
    }

Assessed type

Invalid Validation

c4-pre-sort commented 1 year ago

minhquanym marked the issue as duplicate of #281

c4-judge commented 1 year ago

dmvt marked the issue as unsatisfactory: Invalid