Open code423n4 opened 12 months ago
Picodes marked the issue as primary issue
asselstine marked the issue as sponsor acknowledged
asselstine marked the issue as sponsor confirmed
Adding an internal gas limit then catching the revert would be ideal, so that the claimer won't be griefed and can detect badly-written hooks.
Picodes marked the issue as satisfactory
The issue here is not the possible DoS as claimer can just skip one user, but the possibility of the griefing attack by for example front-running by a malicious hook. I'll give partial credit to duplicates if there is too much focus on DoS.
Picodes marked the issue as selected for report
Lines of code
https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L653 https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L1053 https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L1068
Vulnerability details
Impact
The setHooks function in Vault.sol allows users to set arbitrary hooks, potentially enabling them to make external calls with unintended consequences. This vulnerability could lead to various unexpected behaviors, such as unauthorized side transactions with gas paid unbeknownst to the claimer, reentrant calls, or denial-of-service attacks on claiming transactions.
Vault.sol#L653
Proof of Concept
Consider the following side contract and malicious hook implementation:
Side Contract:
Malicious Hook:
Modified Test File:
When running the test with
forge test --match-test testClaimPrize_viciousHook -vv
the output is:This indicates that it is possible for a hook to make an external call and modify the EVM state. With that fact, attack vectors are multiple.
Tools Used
Foundry
Recommended Mitigation Steps
To prevent any malicious calls there are two possible solutions:
IVaultHook.sol
to set the hooks as view functions and prevent EVM state changes:) external view returns (address);
/// @notice Triggered after the prize pool claim prize function is called. /// @param winner The user who won the prize and for whom this hook is attached @@ -37,5 +37,5 @@ interface IVaultHooks { uint32 prizeIndex, uint256 payout, address recipient
Assessed type
Other