fix: check held balance in rescue operation#1490
Merged
Merged
Conversation
Signed-off-by: jaime-iobermudez <jaime.bermudez@io.builders>
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
marcosio
requested changes
Jun 17, 2026
Signed-off-by: jaime-iobermudez <jaime.bermudez@io.builders>
Signed-off-by: jaime-iobermudez <jaime.bermudez@io.builders>
marcosio
approved these changes
Jun 17, 2026
|
❌ The last analysis has failed. |
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.
Summary
Fixes a Medium-severity vulnerability (reported by the security audit) where the rescue operation could drain stablecoin tokens held in escrow on behalf of token holders.
When a hold is created, the holder's tokens are wiped and an equivalent amount is minted into the stablecoin contract (the treasury) as escrow, tracked by totalHeldAmount. The rescue function only checked the contract's raw token balance and did not subtract the held obligations, so an account with the _RESCUE_ROLE could transfer out the escrow backing active holds — leaving hold records without backing tokens and causing executeHold / releaseHold / reclaimHold to fail.
This aligns rescue with the existing burn logic, which already excludes held amounts via getBurnableAmount().
Changes
Contracts
RescuableFacet.sol: added getRescuableAmount() (balanceOf(contract) − totalHeldAmount) and a checkRescueAmount modifier; rescue() now validates against the rescuable (unreserved) amount instead of the raw contract balance. Registered the new selector.
IRescuable.sol: added the getRescuableAmount() signature and the RescuableAmountExceeded(int64 rescuableAmount) error.
SDK (mirrors the existing burn flow)
New GetRescuableAmountQuery + handler and RescuableAmountExceeded domain error (ErrorCode 20017).
RPCQueryAdapter.getRescuableAmount() calling the new contract view.
ValidationService.checkRescuableAmount() and a pre-flight check in RescueCommandHandler (kept the existing treasury-balance check as well).
Registered the query handler in Injectable.
Tests
hold.test.ts: new "Rescue when held amount greater than 0" block (mirrors the existing burn-with-hold test) — rescuing the full balance with an active hold reverts with RescuableAmountExceeded; rescuing only the unreserved amount succeeds and leaves the escrow intact.
rescuable.test.ts: updated the over-balance case to assert the new RescuableAmountExceeded error.
jest-setup-file.ts: mocked getRescuableAmount.