sec: implement explicit authorization signatures for admin actions - #35
Merged
Conversation
…ixes Kolo-Org#20) - Replaced admin.require_auth() with admin.require_auth_for_args() in add_member and remove_member - Refactored payout() to take expected_recipient and verified authorization for the specific recipient - Enforced strict empty argument payloads for reset_cycle and reset_rotation - Added test_payout_wrong_recipient_auth_fails to verify strict mock_auths validation - Updated tests and test snapshots
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
Thank you for your contribution @Ultra-Tech-code |
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
2 participants
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.
Resolves #20
Description
This PR addresses critical security requirements by enforcing that all admin authorization payloads strictly cover the exact arguments passed to state-modifying actions. This prevents replay attacks or argument spoofing if the contract is used as a cross-contract sub-call, and prepares the contract to be safely managed by multi-sig accounts or other smart contracts.
Changes
add_member&remove_member: Refactored to explicitly useadmin.require_auth_for_args(...)so the signature specifically covers the target member being added or removed.payout: Updated the function signature to take anexpected_recipientargument. The contract asserts this matches the deterministic queue and mandates that the admin payload authorizes payout to this specific recipient.reset_cycle&reset_rotation: Explicitly enforced an empty tuple argument().into_val(&env)for their authorization payloads to ensure tight scoping.payout()with the new signature. Implemented strictmock_auths()payloads and addedtest_payout_wrong_recipient_auth_failswhich ensures a payout fails withError(Auth, InvalidAction)if an admin signed it for a different recipient.Testing
All 20 unit tests pass, and the explicit payload validations have been verified with
cargo test.