fix(contracts): check for sufficient msgValue for AggregationHook
#4673
Conversation
🦋 Changeset detectedLatest commit: b42b871 The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## audit-q3-2024 #4673 +/- ##
================================================
Coverage ? 73.77%
================================================
Files ? 100
Lines ? 1434
Branches ? 187
================================================
Hits ? 1058
Misses ? 355
Partials ? 21
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } | ||
|
|
||
| if (gasRemaining > 0) { | ||
| payable(metadata.refundAddress(msg.sender)).sendValue(gasRemaining); |
There was a problem hiding this comment.
| payable(metadata.refundAddress(msg.sender)).sendValue(gasRemaining); | |
| payable(metadata.refundAddress(message.sender())).sendValue(gasRemaining); |
There was a problem hiding this comment.
you did not make this change...
…messages by making sure msg.value is adequate and refunding if excess.
| @@ -32,16 +44,29 @@ | |||
| ) internal override { | |||
| address[] memory _hooks = hooks(message); | |||
| uint256 count = _hooks.length; | |||
| uint256 valueRemaining = msg.value; | |||
Check notice
Code scanning / Olympix Integrated Security
Local variables in test functions are not properly fuzzed, potentially reducing the effectiveness of property-based testing. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/unfuzzed-local-variables Low
Description
Drive-by changes
Related issues
Backward compatibility
No, needs new deployments of aggregationHooks
Testing
Unit