Skip to content

security: add non_reentrant! RAII macro and enforce CEI on all external calls (Closes #67)#119

Open
neelpote wants to merge 2 commits into
Vero-protocol:mainfrom
neelpote:feat/reentrancy-guards
Open

security: add non_reentrant! RAII macro and enforce CEI on all external calls (Closes #67)#119
neelpote wants to merge 2 commits into
Vero-protocol:mainfrom
neelpote:feat/reentrancy-guards

Conversation

@neelpote

Copy link
Copy Markdown

GrantFox OSS — Official Campaign

Closes #67


Problem

Without a reentrancy guard, any function that makes an external call (token transfer, cross-contract invocation) can be re-entered by a malicious contract before state is written back to storage. The previous implementation used manual reentrancy::lock / reentrancy::unlock calls scattered across vote — 9 call sites, each a potential leak point if a new early-return is added. lock_tokens, resign_guardian, unlock_tokens, and start_reward_stream had no protection at all.


What changed

src/reentrancy.rs

  • Added ReentrancyGuard<'a> RAII struct with a Drop impl that calls unlock automatically on every exit path — normal return, early ?, or panic.
  • Added non_reentrant! macro — one line at the top of any function acquires the lock for its entire scope. No manual unlock ever needed.

src/lib.rs

  • Made reentrancy module pub so tests and external crates can inspect guard state.
  • Replaced all 9 manual reentrancy::lock / reentrancy::unlock calls in vote with non_reentrant!(&env).
  • Applied non_reentrant!(&env) to lock_tokens, resign_guardian, unlock_tokens, and start_reward_stream.
  • Enforced CEI ordering throughout: all storage writes (Effects) happen before any token transfer or cross-contract call (Interaction).

src/task.rs

  • Applied non_reentrant!(env) to register_tasks and cancel_task.

src/drips.rs

  • Moved AllRewardStreams and RewardStream storage writes before env.invoke_contract to enforce CEI.

Tests

8 dedicated reentrancy tests in tests/test.rs:

  • test_strict_reentrancy_guards_prevent_reentry — mock vault re-enters via vote, rejected
  • test_reentrancy_lock_tokens_prevented — malicious token re-enters lock_tokens, rejected
  • test_reentrancy_resign_guardian_prevented — malicious token re-enters resign_guardian, rejected
  • test_reentrancy_unlock_tokens_prevented — malicious token re-enters unlock_tokens, rejected
  • test_reentrancy_start_reward_stream_prevented — malicious drips contract re-enters start_reward_stream, rejected
  • test_lock_released_after_successful_vote — lock releases correctly on success
  • test_lock_released_after_successful_register_task — lock releases correctly on success
  • test_lock_released_after_failed_vote — lock releases correctly on failure

Verification

cargo test
running 55 tests
test result: ok. 55 passed; 0 failed

GrantFox OSS Contribution Guidelines

  • Branch: feat/reentrancy-guards
  • Scoped to single issue #67
  • All new behaviour covered by dedicated tests
  • cargo fmt and cargo clippy clean

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: strict reentrancy protection

1 participant