code-423n4 / 2022-10-paladin-findings

2 stars 0 forks source link

Protocol doesn't work with fee-on-transfer tokens #220

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-10-paladin/blob/main/contracts/WardenPledge.sol#L475 https://github.com/code-423n4/2022-10-paladin/blob/main/contracts/WardenPledge.sol#L508 https://github.com/code-423n4/2022-10-paladin/blob/main/contracts/WardenPledge.sol#L271

Vulnerability details

Impact

Some features won't work with fee-on-transfer tokens. In particular, the following functions will revert because the transfers will exceed the actual balance of the protocol: retrievePledgeRewards, closePledge. Moreover, the rewards issued by pledge function will be less than expected because the receiver pays the token fees. In other cases, the pledge function may revert because the protocol's balance is too low.

The issue is classified as high risk because pledgers can lose tokens. Moreover, some features can become unavailable.

Recovering

It is possible to recover from the DoS state by donating reward tokens to the WardenPledge system. Notice that the receivers of transfers initiated by the protocol still pay the token transfers.

Proof of Concept

https://github.com/code-423n4/2022-10-paladin/blob/main/contracts/WardenPledge.sol#L475 https://github.com/code-423n4/2022-10-paladin/blob/main/contracts/WardenPledge.sol#L508 https://github.com/code-423n4/2022-10-paladin/blob/main/contracts/WardenPledge.sol#L271

Tools Used

Kogaroshi commented 2 years ago

The issue with this type of tokens (and with rebasing tokens) are known, and are the reason why the Pledge contract only accepts tokens that are added to a whitelist (with addRewardToken) as valid tokens to be used for rewards, to prevent any issue when transferring reward tokens. The process to grant the whitelisted status to a token will have to be trusted to the Core team in the beginning, and later on by the Paladin Governance, to make the necessary verifications for each token before adding it the the list.

Kogaroshi commented 2 years ago

duplicate of #27

c4-judge commented 2 years ago

kirk-baird marked the issue as not a duplicate

c4-judge commented 2 years ago

kirk-baird marked the issue as duplicate

c4-judge commented 2 years ago

kirk-baird changed the severity to 2 (Med Risk)

c4-judge commented 2 years ago

kirk-baird changed the severity to QA (Quality Assurance)

c4-judge commented 1 year ago

kirk-baird marked the issue as grade-b