Tapioca DAO - gizzy's results

Lines of code

https://github.com/Tapioca-DAO/tapioca-periph-audit/blob/023751a4e987cf7c203ab25d3abba58f7344f213/contracts/Swapper/UniswapV3Swapper.sol#L38 https://github.com/Tapioca-DAO/tapioca-periph-audit/blob/023751a4e987cf7c203ab25d3abba58f7344f213/contracts/Swapper/UniswapV3Swapper.sol#L184

Vulnerability details

Impact

Uniswapv3swapper uses hardcoded poolFee to make a swap. though it can be changed but is not reliable as swap can happen most often. This can result to a swap being conducted in a pool with little liquidty and with a high price impact.

Proof of Concept

https://github.com/Tapioca-DAO/tapioca-periph-audit/blob/023751a4e987cf7c203ab25d3abba58f7344f213/contracts/Swapper/UniswapV3Swapper.sol#L142C2-L206C6

The swap always uses the state varible poolfee. pool like stable pairs with most liquid pool is 100 fee will excuted at a high price impact rate.

Tools Used

manuel review

Recommended Mitigation Steps

using uniswapv3 should provide poolfee as data or make calculation for most profitable rate.

Assessed type

Uniswap

Lines of code

https://github.com/Tapioca-DAO/tapioca-periph-audit/blob/023751a4e987cf7c203ab25d3abba58f7344f213/contracts/Magnetar/MagnetarV2.sol#L236

Vulnerability details

Impact

In magneter v2 the burst loops and calls all specified calls . when one calls TOFT_WRAP with an msg.value greater than 0 . the burst doubles charges them that amount or will fail if not sent

Proof of Concept

    function burst(
        Call[] calldata calls
    ) external payable returns (Result[] memory returnData) {
        uint256 valAccumulator;

        uint256 length = calls.length;
        returnData = new Result[](length);

        for (uint256 i = 0; i < length; i++) {
            Call calldata _action = calls[i];
            if (!_action.allowFailure) {
                require(
                    _action.call.length > 0,
                    string.concat(
                        "MagnetarV2: Missing call for action with index",
                        string(abi.encode(i))
                    )
                );
            }

            unchecked {
                valAccumulator += _action.value;
            }

            if (_action.id == PERMIT_ALL) {
                _permit(
                    _action.target,
                    _action.call,
                    true,
                    _action.allowFailure
                );
            } else if (_action.id == PERMIT) {
                _permit(
                    _action.target,
                    _action.call,
                    false,
                    _action.allowFailure
                );
            } else if (_action.id == TOFT_WRAP) {
                WrapData memory data = abi.decode(_action.call[4:], (WrapData));
                _checkSender(data.from);
                if (_action.value > 0) {
                    //@audit adds msg.value twice.
                    unchecked {
                        valAccumulator += _action.value;
                    }
                    ITapiocaOFT(_action.target).wrapNative{
                        value: _action.value
                    }(data.to);
                } else {
                    ITapiocaOFT(_action.target).wrap(
                        msg.sender,
                        data.to,
                        data.amount
                    );
                }

The burst funcntion loops and adds each _action.value to valAccumulator at the begining before any call. when TOFT_WRAP if amoung the calls is trigger from if else with _action.value >0 it adds _action.value again to valAccumulator making it bigger and expects at the very end that both eth where sent to call. if not sent it fails. https://github.com/Tapioca-DAO/tapioca-periph-audit/blob/023751a4e987cf7c203ab25d3abba58f7344f213/contracts/Magnetar/MagnetarV2.sol#L194C1-L715C6

  MagnetarV2
       sendFrom()
         should test toft wrap.:
     Error: VM Exception while processing transaction: reverted with reason string 'MagnetarV2: value mismatch'

Tools Used

Manuel review

Recommended Mitigation Steps

remove unchecked { valAccumulator += _action.value; } as it already added it before the call

Assessed type

Error