Skip to content

Commit 1a68114

Browse files
authored
fix: operator precedence bug in UpdateDelegate revoke authority (#253)
Due to Rust's operator precedence (&& binds tighter than ||), the condition `A || B && C && D` was evaluated as `A || (B && C && D)` instead of the intended `(A || (B && C)) && D`. This meant the `plugin.manager() == Authority::UpdateAuthority` check only applied to the additional_delegates branch, not the main resolved_authorities branch. As a result, UpdateDelegate could revoke authority on owner-managed plugins (FreezeDelegate, TransferDelegate) that it should not control. Added explicit parentheses to ensure the manager check applies to both branches.
1 parent 276f2a2 commit 1a68114

File tree

1 file changed

+8
-4
lines changed

1 file changed

+8
-4
lines changed

programs/mpl-core/src/plugins/internal/authority_managed/update_delegate.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -141,15 +141,19 @@ impl PluginValidation for UpdateDelegate {
141141
) -> Result<ValidationResult, ProgramError> {
142142
let plugin = ctx.target_plugin.ok_or(MplCoreError::InvalidPlugin)?;
143143

144-
// If the plugin authority is the authority signing.
145-
if (ctx.resolved_authorities.is_some()
144+
// SECURITY FIX: Added explicit parentheses to fix operator precedence.
145+
// Previously, `&& plugin.manager() == Authority::UpdateAuthority` only bound
146+
// to the additional_delegates branch due to && having higher precedence than ||.
147+
// This allowed UpdateDelegate to revoke authority on owner-managed plugins
148+
// (FreezeDelegate, TransferDelegate) which it should not control.
149+
if ((ctx.resolved_authorities.is_some()
146150
&& ctx
147151
.resolved_authorities
148152
.unwrap()
149153
.contains(ctx.self_authority))
150154
// Or the authority is one of the additional delegates.
151-
|| (self.additional_delegates.contains(ctx.authority_info.key) && PluginType::from(plugin) != PluginType::UpdateDelegate)
152-
// And it's an authority-managed plugin.
155+
|| (self.additional_delegates.contains(ctx.authority_info.key) && PluginType::from(plugin) != PluginType::UpdateDelegate))
156+
// And it's an authority-managed plugin (applies to BOTH branches).
153157
&& plugin.manager() == Authority::UpdateAuthority
154158
{
155159
approve!()

0 commit comments

Comments
 (0)