code-423n4 / 2022-02-skale-findings

0 stars 0 forks source link

Not compatible with Rebasing/Deflationary/Inflationary tokens #50

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/DepositBoxes/DepositBoxERC20.sol#L299-L308

Vulnerability details

Proof of Concept

The DepositBoxERC20 contract do 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.

Recommended Mitigation Steps

Add support in contracts for such tokens before accepting user-supplied tokens Consider to check before/after balance on the vault.

cstrangedk commented 2 years ago

Reference https://github.com/code-423n4/2022-02-skale-findings/issues/8, https://github.com/code-423n4/2022-02-skale-findings/issues/9, https://github.com/code-423n4/2022-02-skale-findings/issues/10, https://github.com/code-423n4/2022-02-skale-findings/issues/12, https://github.com/code-423n4/2022-02-skale-findings/issues/19 #42

Only standard functions are applied to out-of-the-box DepositBoxes and TokenManagers. It's up to the SKALE chain owner's discretion to create custom DepositBoxes/TokenManagers that specifically support non-standard functions like Rebasing/Deflationary/Inflationary tokens, and add these custom contracts to the bridge using registerExtraContract() functions in MessageProxy.

GalloDaSballo commented 2 years ago

Because this is reliant on configuration, I believe the finding to be valid and of medium severity.

End users can verify if the DepositBoxes are properly handling rebasing tokens at the time they wish to bridge