Oracle: Make permissions account mandatory for all privileged instructions #388
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jayantk
previously approved these changes
Nov 17, 2023
| permissions_account_data.master_authority = *funding_account.key; | ||
| permissions_account_data.data_curation_authority = *funding_account.key; | ||
| permissions_account_data.security_authority = *funding_account.key; | ||
| } |
There was a problem hiding this comment.
can you test that the old way of invoking these instructions fails now?
There was a problem hiding this comment.
err also would be good to test the specific authority that is allowed to do these operations. So maybe set that the other authorities are different
(unless this is already tested elsewhere, which i think it probably is)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
During an audit of oracle code, Amin and I discovered that it is still possible to circumvent the new permissions PDA mechanism if you simply don't pass the permissions account. If you as an attacker have control over price/product/mapping private keys, you can use the legacy privkey-based access control, which means the oracle has a significantly larger attack surface than necessary. This change makes the permissions account mandatory and adjusts all affected unit tests.
Summary of changes
program/rust/src/tests/pyth_simulator.rs- always use permissions account in instruction wrappersprogram/rust/src/utils.rs- Return error on missing permissions accountprogram/rust/src/tests/- Addpermissions_account_setupharness assignments to all affected tests.Review Highlights
program/rust/src/tests/- Some of the assertions were removed, as they were testing the legacy privkey access control which is no longer available. That said, it's important that useful assertions were not removed.