feat: Add S030 rule to detect require_auth instead of require_auth_for_args#911
Open
meshackyaro wants to merge 1 commit into
Open
feat: Add S030 rule to detect require_auth instead of require_auth_for_args#911meshackyaro wants to merge 1 commit into
meshackyaro wants to merge 1 commit into
Conversation
|
@meshackyaro is attempting to deploy a commit to the gbangbolaoluwagbemiga's projects Team on Vercel. A member of the Team first needs to authorize it. |
…r_args Implements detection for replay/scope-confusion attacks on multi-arg admin operations. - Add S029 finding code for require_auth_for_args violations - Implement RequireAuthForArgsRule with comprehensive detection logic - Add rule documentation with attack scenarios and mitigation strategies - Add test fixtures with vulnerable and safe code examples - Add 6 unit tests covering all edge cases - Add 2 CLI integration tests for JSON and NDJSON output - Update fixtures README to include S029 (and missing S025-S027) The rule flags public functions with 2+ Address parameters that use require_auth() instead of require_auth_for_args() when performing sensitive operations (storage mutations or external calls). Closes HyperSafeD#753
a8e49bd to
13b7c78
Compare
|
@meshackyaro Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
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
This PR implements the S030 security rule to detect when
require_auth()is used instead ofrequire_auth_for_args()in Soroban smart contracts with multiple Address parameters, which enables replay/scope-confusion attacks on multi-arg admin operations.Closes #753
Changes
Core Implementation
REQUIRE_AUTH_FOR_ARGSconstant with High severityRequireAuthForArgsRuleintooling/sanctifier-core/src/rules/require_auth_for_args.rsrequire_auth()instead ofrequire_auth_for_args()Documentation
docs/rules/require-auth-for-args.mdincluding:Testing
require_auth_for_args()usagecontracts/fixtures/finding-codes/s030_require_auth_for_args.rsAdditional Updates
contracts/fixtures/finding-codes/README.mdto include S030Cargo.lockwith dependenciesSecurity Impact
Vulnerability Prevented
Replay/Scope-Confusion Attacks: When a function has multiple
Addressparameters (e.g.,set_admin(caller, new_admin)), usingrequire_auth()only verifies thatcallersigned something, but not what they signed. An attacker can:Example Attack Scenario
Attack:
set_admin(alice, bob)to make Bob adminset_admin(alice, attacker_address)alice.require_auth()only checks Alice signedMitigation
Testing
All tests pass:
Checklist