-
Notifications
You must be signed in to change notification settings - Fork 12
Description
Description
The config_buffer module is used to store config changes that should take effect in the next epoch.
Trusted modules like consensus_config, execution_config, gas_schedule and AutomationRegistryConfig, etc. use this buffer to schedule updates. They add new configs using the upsert() function and later apply them using the config_buffer::extract() function during the on_new_epoch() call.
The problem is that config_buffer::extract() is marked as public, which means that anyone can call it - not just
trusted modules. If an attacker calls config_buffer::extract<T>() before the real reconfiguration happens, they remove the pending config from the buffer. Then, when the trusted module checks if the config exists
during on_new_epoch(), it won’t find anything, and the update will be silently skipped.
Preliminary investigation
All config change updates need to be immediately followed up with a call to supra_framework::supra_governance::reconfigure(). This is because the config update(any config update) is processed only as part of reconfiguration_with_dkg::finish() and this is triggered only by supra_framework::supra_governance::reconfigure().
Essentially, after performing a config update, we will compulsorily have to call supra_framework::supra_governance::reconfigure() because that is the only place where any config update is processed and applied. This will ensure that the config updates are applied immediately after updating the buffer. Nonetheless, once we integrate DKG we will be moving to reconfiguration_with_dkg::finish() meaning config updates will happen at epoch boundaries, which will then mean we need to be careful of this bug.
Recommendation
The config_buffer::extract() function is currently not signer-gated nor is a friend function, which then allows public users to call the this without any restriction. If changed to a friend function, only the specific friend modules can call.