Putty contest - TrungOre's results

Lines of code

https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L500

Vulnerability details

Impact

user can't withdraw their asset

Proof of concept

Now there are no checks for the feeAmount to be transfered via function withdraw(). As baseAsset can be an arbitrary token, in the case when such token doesn't allow for zero amount transfers and feeAmount = 0, the fee and also the asset can become unavailable. (tx revert)

if ((order.isCall && isExercised) || (!order.isCall && !isExercised)) {
    // send the fee to the admin/DAO if fee is greater than 0%
    uint256 feeAmount = 0;
    if (fee > 0) {
        feeAmount = (order.strike * fee) / 1000;
        /// feeAmount = 0 --> transfer fail 
        ERC20(order.baseAsset).safeTransfer(owner(), feeAmount);
    }


    ERC20(order.baseAsset).safeTransfer(msg.sender, order.strike - feeAmount);


    return;
}

Of course, if the amount of baseToken is high enough, this loss can't happen. But assume that there is a token with low precision and high price (precision = 1,2 and token value is very high like BTC), this issue will be noticeable.

Tools Used

Manual review

Recommended Mitigation Steps

add check feeAmount > 0 before transfer

if ((order.isCall && isExercised) || (!order.isCall && !isExercised)) {
    // send the fee to the admin/DAO if fee is greater than 0%
    uint256 feeAmount = 0;
    if (fee > 0) {
        feeAmount = (order.strike * fee) / 1000;
        if (feeAmount > 0) { /// add check here 
            ERC20(order.baseAsset).safeTransfer(owner(), feeAmount);    
        }
    }

    ERC20(order.baseAsset).safeTransfer(msg.sender, order.strike - feeAmount);

    return;
}

[2022-06-putty] QA report

tags: c4, 2022-06-putty, QA

no need to use statement return

• https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L518

use external instead of public

• https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L466
• https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L389

remove payable

• https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L228
• https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L240

no need to check token.code.length > 0

• https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L598
• https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L293