Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integer overflow in oracle pallet in spacewalk #288

Open
prayagd opened this issue Jul 31, 2023 · 3 comments
Open

Integer overflow in oracle pallet in spacewalk #288

prayagd opened this issue Jul 31, 2023 · 3 comments
Labels
priority:low Do it some day type:bug Something isn't working

Comments

@prayagd
Copy link
Collaborator

prayagd commented Jul 31, 2023

Context

Issue found by SRL labs in the semi-automated audit.

Summary

An integer overflow in the oracle pallet can be abused by a malicious oracle.

Issue details

There is an integer overflow inside the oracle::begin_block function which is called upon block initialization. A malicious oracle can trigger this overflow by updating the coin info with high supply and price values via set_updated_coin_infos call inside Pendulum's dia-oracle pallet.

Here is an example call parameters that will trigger the overflow in the next block initialization:

RuntimeCall::DiaOracleModule(Call::set_updated_coin_infos {
    coin_infos: [(
        ([0], [0]),
        CoinInfo {
            symbol: [],
            name: [0],
            blockchain: [],
            supply: 45172881575663848363994640109535494224,
            last_update_timestamp: 60000533389444330,
            price: 338974337383797358236404514952583315520,
        })]
});

Risk

By triggering this integer overflow, a malicious oracle can:

Crash the nodes compiled in debug mode with overflow checks enabled
On nodes which have overflow checks disabled, unexpected behaviors and logic inconsistencies
We assigned a severity of low to this issue since it can only be triggered by permissioned oracles.

Mitigation

Implement proper integer overflow handling by checking call arguments and using safe arithmetic functions.

@prayagd prayagd added the priority:low Do it some day label Jul 31, 2023
@prayagd
Copy link
Collaborator Author

prayagd commented Jul 31, 2023

@prayagd
Copy link
Collaborator Author

prayagd commented Nov 30, 2023

@ebma do you still think this is low priority? if yes should i move i icebox if it holds future relevance?

@ebma
Copy link
Member

ebma commented Dec 4, 2023

Yes, it only would have a higher priority once we decide to let a third party have an authorized account that is allowed to feed price info to our chain. Since it's a security issue we should fix it before this happens. But as long as it's only us feeding the price info, there is no problem.

@TorstenStueber TorstenStueber added the type:bug Something isn't working label Dec 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority:low Do it some day type:bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants