code-423n4 / 2023-01-rabbithole-findings

1 stars 2 forks source link

Tokens with fee on transfer are not supported #662

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/Erc20Quest.sol#L67 https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/Erc20Quest.sol#L86 https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/Erc20Quest.sol#L103

Vulnerability details

There are several ERC20 tokens that charge a transfer fee eg. STA, whereas some do not currently charge a fee but may do so in the future e.g. USDC.

The protocol does not appear to support rebasing/deflationary/inflationary tokens whose balance changes during transfers or over time. The necessary checks include at least verifying the amount of tokens transferred to contracts before and after the actual transfer to infer any fees/interest.

Proof of Concept

https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/Erc20Quest.sol#L67 https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/Erc20Quest.sol#L86 https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/Erc20Quest.sol#L103

Recommended Mitigation Steps

c4-judge commented 1 year ago

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

waynehoover commented 1 year ago

We do not plan on supporting rebasing tokens.

c4-sponsor commented 1 year ago

waynehoover marked the issue as sponsor acknowledged

c4-judge commented 1 year ago

This previously downgraded issue has been upgraded by kirk-baird

c4-judge commented 1 year ago

This previously downgraded issue has been upgraded by kirk-baird

c4-judge commented 1 year ago

kirk-baird marked the issue as duplicate of #630

c4-judge commented 1 year ago

kirk-baird marked the issue as satisfactory

c4-judge commented 1 year ago

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