Paladin - Warden Pledges contest - peritoflores's results

Lines of code

https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L653-L661 https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L585-L592

Vulnerability details

Impact

Malicious owner can steal any created pledge even to drain the whole contract

Proof of Concept

Functions like recoverERC20 are good to recover tokens accidentally transferred to a contract.

The common approach for these function is to exclude real tokens handled by the protocol otherwise admin can just rug pull the contract.

In your case, as admin can add/remove reward tokens with the funcion removeRewardToken, he can call this function first to bypass the require statement in recoverERC20

function stealToken(address tokenToSteal){
    removeRewardToken(tokenToSteal);
    recoverERC20(tokenToSteal);
}

Recommended

One way to solve this is to keep a balance of every token and let owner to transfer the difference (if it exists), though this solution will cost some gas.

Lines of code

https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L335 https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L396

Vulnerability details

Impact

Users are unable neither to createPleadge nor extendPledge if protocol Fees are set to zero for some tokens

Proof of Concept

It is not stated in documentation if you allow protocol fees to zero. For your code it is possible to owner to set the fee to zero.

 function updatePlatformFee(uint256 newFee) external onlyOwner {
    if(newFee > 500) revert Errors.InvalidValue();
    uint256 oldfee = protocalFeeRatio;
    protocalFeeRatio = newFee;
    emit PlatformFeeUpdated(oldfee, newFee);
}

However there are many non-malicious tokens that will revert if you try to transfer zero tokens. For these cases the function createPledge and extendPledge will revert when trying to transfer fees to chest

IERC20(rewardToken).safeTransferFrom(creator, chestAddress, vars.feeAmount);  @audit will revert if fee is zero

An scenario is that some user create a pledge, then the owner decides to set fee to zero for some reason. That user will be unable to extend pledge and pledge will expiry.

Recommended

If you do not plan to set fee to zero it is better to set a minimum fee.