code-423n4 / 2024-02-tapioca-findings

1 stars 1 forks source link

Unable to decode the `duration` parameter in the function `TapTokenC::decodeLockTwpTapDstMsg()` #144

Closed c4-bot-8 closed 3 months ago

c4-bot-8 commented 4 months ago

Lines of code

https://github.com/Tapioca-DAO/tap-token/blob/20a83b1d2d5577653610a6c3879dff9df4968345/contracts/tokens/TapTokenCodec.sol#L75

Vulnerability details

Description

The function decodeLockTwpTapDstMsg() is utilized to decipher an encoded message intended for the lockTwTapPosition() operation.

https://github.com/Tapioca-DAO/tap-token/blob/20a83b1d2d5577653610a6c3879dff9df4968345/contracts/tokens/TapTokenCodec.sol#L62-L81

function decodeLockTwpTapDstMsg(bytes memory _msg)
    internal
    pure
    returns (LockTwTapPositionMsg memory lockTwTapPositionMsg_)
{
    // TODO bitwise operators
    // Offsets
    uint8 userOffset_ = 20;
    uint8 durationOffset_ = 32;

    // Decoded data
    address user = BytesLib.toAddress(BytesLib.slice(_msg, 0, userOffset_), 0);

    uint96 duration = BytesLib.toUint96(BytesLib.slice(_msg, userOffset_, durationOffset_), 0);

    uint256 amount = BytesLib.toUint256(BytesLib.slice(_msg, durationOffset_, _msg.length - durationOffset_), 0);

    // Return structured data
    lockTwTapPositionMsg_ = LockTwTapPositionMsg(user, duration, amount);
}

The input _msg is encoded using the abi.encodePacked() operation with three parameters: user of type address, duration of type uint96, and amount of type uint256. Thus, the value of each parameter can be obtained by extracting the bytes within specific ranges of the _msg message:

To achieve this, the function employs the BytesLib.slice() function from the BytesLib library to extract the byte ranges of the _msg.

https://github.com/GNSPS/solidity-bytes-utils/blob/df88556cbbc267b33a787a3a6eaa32fd7247b589/contracts/BytesLib.sol#L228-L232

function slice(
    bytes memory _bytes,
    uint256 _start,
    uint256 _length
) 
    internal
    pure
    returns (bytes memory)

However, there's an issue when handling the duration parameter, which is a uint96 variable (96 bits = 12 bytes). The function erroneously uses durationOffset_ = 32 as the _length parameter when passing it to the BytesLib.slice() function. Consequently, when the casting BytesLib.toUint96() attempts to convert a 32-byte value into a 12-byte value, it triggers a revert.

Impact

The function _lockTwTapPositionReceiver fails to execute as the transaction reverts.

Tools Used

Manual review

Recommended Mitigation Steps

When decoding the value of the variable duration in the function TapTokenC::decodeLockTwpTapDstMsg, set _length to 12

uint96 duration = BytesLib.toUint96(BytesLib.slice(_msg, userOffset_, 12), 0);

Assessed type

en/de-code

c4-sponsor commented 4 months ago

cryptotechmaker (sponsor) confirmed

c4-judge commented 3 months ago

dmvt marked the issue as primary issue

c4-judge commented 3 months ago

dmvt marked the issue as selected for report

c4-judge commented 3 months ago

dmvt marked the issue as not selected for report

c4-judge commented 3 months ago

dmvt marked the issue as satisfactory

c4-judge commented 3 months ago

dmvt marked issue #69 as primary and marked this issue as a duplicate of 69