[stable2512] Backport #11612#11637
Merged
EgorPopelyaev merged 3 commits intostable2512from Apr 10, 2026
Merged
Conversation
Author
|
Please cherry-pick the changes locally and resolve any conflicts. git fetch origin backport-11612-to-stable2512
git worktree add --checkout .worktree/backport-11612-to-stable2512 backport-11612-to-stable2512
cd .worktree/backport-11612-to-stable2512
git reset --hard HEAD^
git cherry-pick -x ecada3402a70d906e10c6d33b0f42b6174fea119
git push --force-with-lease |
4 tasks
Contributor
|
@dhirajs0 Hey Dhiraj, couls you check conflicts here? |
…when parent bounty is not Active (#11612) Fix an authorization bypass in `pallet-multi-asset-bounties` where any signed account could forcibly unassign an active child bounty's curator when the parent bounty was not in `Active` state (e.g., `CuratorUnassigned`). This also caused the child curator's native balance hold (deposit) to be permanently leaked — removed from pallet storage but never released or burned on-chain. **Root cause:** In `unassign_curator`, the `BountyStatus::Active` branch's catch-all `Some(sender)` arm used `if let Some(parent_curator) = parent_curator { ... }` with no `else` clause. When `parent_curator` was `None` (parent bounty not Active), the block was silently skipped and execution fell through to the state transition — no `BadOrigin` error was returned. No integration changes required for downstream projects. This is a fix internal to `pallet-multi-asset-bounties` with no public API changes. The extrinsic signature and behavior for authorized callers remain identical. The fix restructures the `BountyStatus::Active` arm in `unassign_curator` with two changes: Previously, `CuratorDeposit::take()` was called unconditionally at the top of the `Active` arm (before verifying the caller). Now it is called inside each `match maybe_sender` arm, only after the caller is confirmed to be authorized. This prevents the deposit from being removed from storage on an unauthorized (and reverted) call path. ```diff BountyStatus::Active { ref curator, .. } => { - let maybe_curator_deposit = - CuratorDeposit::<T, I>::take(parent_bounty_id, child_bounty_id); match maybe_sender { None => { - if let Some(curator_deposit) = maybe_curator_deposit { + if let Some(curator_deposit) = + CuratorDeposit::<T, I>::take(parent_bounty_id, child_bounty_id) + { T::Consideration::burn(curator_deposit, curator); } }, ``` The catch-all `Some(sender)` arm now uses `parent_curator.ok_or(BadOrigin)?` followed by an `ensure!`. When `parent_curator` is `None`, the call is immediately rejected with `BadOrigin`. ```diff Some(sender) => { - if let Some(parent_curator) = parent_curator { - if sender == parent_curator && *curator != parent_curator { - if let Some(curator_deposit) = maybe_curator_deposit { - T::Consideration::burn(curator_deposit, curator); - } - } else { - return Err(BadOrigin.into()); - } + let parent_curator = parent_curator.ok_or(BadOrigin)?; + ensure!( + sender == parent_curator && *curator != parent_curator, + BadOrigin + ); + if let Some(curator_deposit) = + CuratorDeposit::<T, I>::take(parent_bounty_id, child_bounty_id) + { + T::Consideration::burn(curator_deposit, curator); } }, ``` A comprehensive test (`unprivileged_caller_cannot_unassign_active_child_curator_when_parent_not_active`) is added that: 1. Creates an active child bounty with a separate child curator. 2. Has the parent curator voluntarily unassign (putting parent into `CuratorUnassigned`). 3. Asserts that an unprivileged attacker is rejected with `BadOrigin`. 4. Verifies the child bounty remains `Active`, the curator deposit stays in storage, and the balance hold is intact. 5. Confirms the child curator can still voluntarily unassign themselves and that the deposit is properly released. * [x] My PR includes a detailed description as outlined in the "Description" and its two subsections above. * [x] My PR follows the [labeling requirements] * [x] I have made corresponding changes to the documentation (if applicable) * [x] I have added tests that prove my fix is effective or that my feature works (if applicable) --------- Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com> (cherry picked from commit ecada34)
06599d6 to
c8ba830
Compare
dhirajs0
approved these changes
Apr 8, 2026
Contributor
|
This pull request is amending an existing release. Please proceed with extreme caution,
Emergency Bypass
If you really need to bypass this check: add |
Szegoo
approved these changes
Apr 8, 2026
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
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.
Backport #11612 into
stable2512from dhirajs0.See the documentation on how to use this bot.