Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(contracts): quote management for L2->L1 hooks #4552

Merged
merged 56 commits into from
Oct 31, 2024

Conversation

aroralanuk
Copy link
Contributor

@aroralanuk aroralanuk commented Sep 23, 2024

Description

  • check for sufficient fees in AbstractMessageIdAuthHook and refund surplus
  • add a child hook to OPL2ToL1Hook and ArbL2ToL1Hook to use the igp to pay for the destination gas fees

Note: LayerzeroL2Hook currently also refunds from msg.value, will make it into issue to be fixed later as we're using the layerzero hooks right now.

Drive-by changes

  • None

Related issues

Backward compatibility

No

Testing

Fuzz

@aroralanuk aroralanuk changed the base branch from main to kunal/HL2408-002-fix September 26, 2024 07:56
Base automatically changed from kunal/HL2408-002-fix to main October 22, 2024 07:22
// Immutable quote amount
uint256 public immutable GAS_QUOTE;
// child hook to call first
IPostDispatchHook public immutable childHook;

Check notice

Code scanning / Olympix Integrated Security

Some state variables are not being fuzzed in test functions, potentially leaving vulnerabilities unexplored. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/unfuzzed-variables Low

Some state variables are not being fuzzed in test functions, potentially leaving vulnerabilities unexplored. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/unfuzzed-variables
solidity/contracts/hooks/ArbL2ToL1Hook.sol Dismissed Show dismissed Hide dismissed
// Immutable quote amount
uint32 public immutable GAS_QUOTE;
// child hook to call first
IPostDispatchHook public immutable childHook;

Check notice

Code scanning / Olympix Integrated Security

Some state variables are not being fuzzed in test functions, potentially leaving vulnerabilities unexplored. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/unfuzzed-variables Low

Some state variables are not being fuzzed in test functions, potentially leaving vulnerabilities unexplored. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/unfuzzed-variables
require(
msg.value >= metadata.msgValue(0) + GAS_QUOTE,
"OPL2ToL1Hook: insufficient msg.value"
bytes memory payload = abi.encodeCall(

Check warning

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 Medium

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
require(
metadata.msgValue(0) < 2 ** 255,
"OPStackHook: msgValue must be less than 2 ** 255"
bytes memory payload = abi.encodeCall(

Check warning

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 Medium

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
@@ -72,7 +78,9 @@
options,
false // payInLzToken
);
lZEndpoint.send{value: msg.value}(msgParams, refundAddress);

uint256 quote = _quoteDispatch(metadata, message);

Check warning

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 Medium

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
lZEndpoint.send{value: msg.value}(msgParams, refundAddress);

uint256 quote = _quoteDispatch(metadata, message);
lZEndpoint.send{value: quote}(msgParams, refundAddress);

Check warning

Code scanning / Olympix Integrated Security

Calling a function without checking the return value may lead to silent failures. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/unused-return-function-call Medium

Calling a function without checking the return value may lead to silent failures. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/unused-return-function-call
lZEndpoint.send{value: msg.value}(msgParams, refundAddress);

uint256 quote = _quoteDispatch(metadata, message);
lZEndpoint.send{value: quote}(msgParams, refundAddress);

Check warning

Code scanning / Olympix Integrated Security

Using send() without checking the return value may lead to silent failures of ether transmittal. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/unchecked-send Medium

Using send() without checking the return value may lead to silent failures of ether transmittal. For more information, visit: http://detectors.olympixdevsectools.com/article/web3-vulnerability/unchecked-send

_sendMessageId(metadata, message);

uint256 _overpayment = msg.value - _quoteDispatch(metadata, message);

Check warning

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 Medium

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

uint256 _overpayment = msg.value - _quoteDispatch(metadata, message);
if (_overpayment > 0) {
address _refundAddress = metadata.refundAddress(

Check warning

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 Medium

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
@aroralanuk aroralanuk added this pull request to the merge queue Oct 31, 2024
Merged via the queue into main with commit 469f2f3 Oct 31, 2024
36 checks passed
@aroralanuk aroralanuk deleted the kunal/check-suff-refund-extra branch October 31, 2024 01:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants