-
Notifications
You must be signed in to change notification settings - Fork 1
Closed
Labels
LowLow severity security findingLow severity security finding⎈ QuantStampQuantStamp audit findingQuantStamp audit finding
Description
Severity: Low
Files Affected
solidity/src/FlowYieldVaultsRequests.sol
Description
In completeProcessing() in FlowYieldVaultsRequests, the authorized COA supplies a yieldVaultId and the contract sets request.yieldVaultId = yieldVaultId. On successful CREATE_YIELDVAULT, it calls _registerYieldVault() to mark the ID valid and assign local ownership, and on successful CLOSE_YIELDVAULT it calls _unregisterYieldVault() to remove local ownership and invalidate the ID.
However:
- The contract does not defensively bind the completion-time yieldVaultId to the request's originally intended yieldVaultId for non-CREATE flows
_registerYieldVault()does not enforce that yieldVaultId is not already registered_unregisterYieldVault()clears global registry state even if the array-level "swap-and-pop" safety check fails
This could lead to COA operational mistakes corrupting the EVM-side registry view by overwriting ownership, creating duplicate entries, or invalidating an unrelated YieldVault ID.
Example (misbound CLOSE unregisters the wrong vault)
- Alice owns YieldVaultId
7and Bob owns YieldVaultId8(both registered on EVM). - Alice submits a
CLOSE_YIELDVAULTrequest for7. Cadence closes7successfully. - Authorized COA mistake: calls EVM
completeProcessing(..., success=true, yieldVaultId=8, ...)for Alice’s request. - EVM runs
_unregisterYieldVault(8, Alice, ...); even if the user-array removal check fails, it still clearsvalidYieldVaultIds[8]and deletesyieldVaultOwners[8]. - Result: Bob’s vault
8is removed from the EVM registry even though it still exists on Cadence, breaking Bob’s future EVM interactions.
Recommendation
- Enforce strong ID binding in
completeProcessing()for DEPOSIT/WITHDRAW/CLOSE by requiring the supplied yieldVaultId matches the request's stored yieldVaultId - Add a uniqueness guard in
_registerYieldVault()(reject registering an already-valid yieldVaultId) - Only clear
validYieldVaultIds/yieldVaultOwnersin_unregisterYieldVault()after proving the YieldVault is actually registered under the provided user
Parent Issue: #15
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
LowLow severity security findingLow severity security finding⎈ QuantStampQuantStamp audit findingQuantStamp audit finding