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

Fix - Invoke non program account owned by a builtin #5158

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Lichtso
Copy link

@Lichtso Lichtso commented Mar 5, 2025

Problem

Currently there is a slight bug once remove_accounts_executable_flag_checks is active:
One can invoke a built-in program by invoking any account owned by it instead. This leads to the built-in running as a different pubkey, thus all ownership checks fail and the built-in has no write access to anything.

This is benign as it only allows invoking built-in programs in a strange nonsensical way. But, it is still a stupid thing to support and would hinder future protocol changes such as these in the account loader.

About rekeying: Well see Discord discussion.

Summary of Changes

Blocks the execution of any non loader owned or non built-in account and adds test_invoke_non_program_account_owned_by_a_builtin() to demonstrate the change in behavior.

Feature Gate Issue: https://github.com/anza-xyz/feature-gate-tracker/issues/69

@Lichtso Lichtso requested review from jstarry and 2501babe March 5, 2025 19:31
@Lichtso Lichtso force-pushed the fix/invoke_non_program_account_owned_by_a_builtin branch from 1738f1c to ad7759a Compare March 5, 2025 19:42
@Lichtso Lichtso force-pushed the fix/invoke_non_program_account_owned_by_a_builtin branch from ad7759a to 64d3113 Compare March 5, 2025 20:58
@@ -13556,15 +13596,15 @@ fn test_loader_v3_to_v4_migration() {
finalized_migration_transaction.clone(),
Err(TransactionError::InstructionError(
0,
InstructionError::InvalidInstructionData,
InstructionError::UnsupportedProgramId,
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where I first noticed this issue.

@Lichtso Lichtso marked this pull request as ready for review March 6, 2025 20:52
@Lichtso Lichtso requested a review from a team as a code owner March 6, 2025 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants