Skip to content

Latest commit

 

History

History
66 lines (38 loc) · 1.71 KB

File metadata and controls

66 lines (38 loc) · 1.71 KB

Micro Wooden Meerkat

Medium

_entrancyCounter Reentry check can be bypassed by modifying Safe.nonce value

Summary

HSG has safeguards to prevent signers and modules changing the Safe state. For the checkTransaction flow it uses two layer approach:

  1. Whitelist of enabledDelegatecallTargets[].

  2. On execution, HSG takes a snapshot of the safe state, and afterwards at checkAfterExecution -> _checkSafeState it verifies the state has not been tampered with.

The 2nd layer can be bypassed.

Root Cause

HSG team correctly identified that a delegate call can invoke checkTransaction outside of the regular Safe flow, and in order to mitigate it they track Safe.nonce value:

https://github.com/sherlock-audit/2024-11-hats-protocol/blob/main/hats-zodiac/src/HatsSignerGate.sol#L430

we rely on the invariant that the Safe nonce increments every time Safe.execTransaction calls out to IGuard.checkTransaction.

As such the following is mitigated:

execTransaction -> checkTransaction
add owner
direct call checkTransaction - to update the snapshot

The direct call fails since Safe.nonce has not been incremented.

However the issue is that a delegated call can modify any Safe value, including the nonce, thus the attack becomes:

execTransaction -> checkTransaction
add owner
modify nonce slot
direct call checkTransaction - to update the snapshot

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

delegatecalls can modify critical Safe parameters.

PoC

It's a complicated POC and the contest ends soon, I will add one in the comments during judging phase if required.

Mitigation

No response