c4-bot-2 commented 7 months ago

Lines of code

https://github.com/code-423n4/2024-03-acala/blob/9c71c05cf2d9f0a2603984c50f76fc8a315d4d65/src/orml/rewards/src/lib.rs#L157 https://github.com/code-423n4/2024-03-acala/blob/9c71c05cf2d9f0a2603984c50f76fc8a315d4d65/src/orml/rewards/src/lib.rs#L179 https://github.com/code-423n4/2024-03-acala/blob/9c71c05cf2d9f0a2603984c50f76fc8a315d4d65/src/orml/rewards/src/lib.rs#L216 https://github.com/code-423n4/2024-03-acala/blob/9c71c05cf2d9f0a2603984c50f76fc8a315d4d65/src/orml/rewards/src/lib.rs#L273

Vulnerability details

Impact

DoS can be achieved over principal flows like adding shares, removing shares, claiming rewards and so on... thanks to an iteration over a mapper that holds no entries boundaries.

Proof of Concept

PoolInfos and SharesAndWithdrawnRewards mappers holds the info about CurrencyId and Balances in a BTreeMap as there could be different balance amount for each currency, however in the flows of add_share, remove_share, claim_rewards and others the logic iterates over all the entries, if the numbers of entries will be to big it will cause the main functions we iterate over above to fail with out of gas as there are is not enough resources to iterate over all of them.

Tools Used

Manual Review

Recommended Mitigation Steps

Limit the number of Currency's the mappers can hold in the BTree.

Assessed type

DoS

c4-pre-sort commented 7 months ago

DadeKuma marked the issue as duplicate of #62

DadeKuma commented 7 months ago

Has the same root cause as the main dup

c4-pre-sort commented 7 months ago

DadeKuma marked the issue as sufficient quality report

c4-judge commented 7 months ago

OpenCoreCH changed the severity to QA (Quality Assurance)