-
Notifications
You must be signed in to change notification settings - Fork 369
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
Changes from all commits
1560f5c
44379e1
d680e50
ca26e88
a5c4b62
54b4327
66c3bb7
e98382d
56ea53c
30982d2
88cb33f
baef8ff
9684835
6c31fbf
f64611d
ac3613e
4d293ea
fb66306
13c61fd
fb089e2
c70ac54
e6c5b10
08dda88
37ecb27
1dd0fc3
c99eb24
97b7bc8
c6b5f76
bef3470
541be43
4233c0a
41a407d
5610c33
ef780c6
a6731b0
47defbf
8510b6a
a1a850e
7ba121d
101fe72
cc69d2e
a0aaf23
5d90cfe
43eba79
750d09b
5991022
71d23f4
8df9105
9093369
56c6b1d
01247ce
e16958e
64e7628
1ed932d
255f598
5edc95a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
--- | ||
'@hyperlane-xyz/sdk': minor | ||
'@hyperlane-xyz/core': minor | ||
--- | ||
|
||
Checking for sufficient fees in `AbstractMessageIdAuthHook` and refund surplus |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
--- | ||
"@hyperlane-xyz/utils": patch | ||
'@hyperlane-xyz/utils': patch | ||
--- | ||
|
||
Filter undefined/null values in invertKeysAndValues function |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,10 +14,13 @@ | |
@@@@@@@@@ @@@@@@@@*/ | ||
|
||
// ============ Internal Imports ============ | ||
import {Message} from "../libs/Message.sol"; | ||
import {AbstractPostDispatchHook, AbstractMessageIdAuthHook} from "./libs/AbstractMessageIdAuthHook.sol"; | ||
import {AbstractMessageIdAuthorizedIsm} from "../isms/hook/AbstractMessageIdAuthorizedIsm.sol"; | ||
import {StandardHookMetadata} from "./libs/StandardHookMetadata.sol"; | ||
import {TypeCasts} from "../libs/TypeCasts.sol"; | ||
import {IPostDispatchHook} from "../interfaces/hooks/IPostDispatchHook.sol"; | ||
import {InterchainGasPaymaster} from "./igp/InterchainGasPaymaster.sol"; | ||
|
||
// ============ External Imports ============ | ||
import {ICrossDomainMessenger} from "../interfaces/optimism/ICrossDomainMessenger.sol"; | ||
|
@@ -30,13 +33,16 @@ | |
*/ | ||
contract OPL2ToL1Hook is AbstractMessageIdAuthHook { | ||
using StandardHookMetadata for bytes; | ||
using Message for bytes; | ||
|
||
// ============ Constants ============ | ||
|
||
// precompile contract on L2 for sending messages to L1 | ||
ICrossDomainMessenger public immutable l2Messenger; | ||
// Immutable quote amount | ||
uint32 public immutable GAS_QUOTE; | ||
// child hook to call first | ||
IPostDispatchHook public immutable childHook; | ||
aroralanuk marked this conversation as resolved.
Show resolved
Hide resolved
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
|
||
// Minimum gas limit that the message can be executed with - OP specific | ||
uint32 public constant MIN_GAS_LIMIT = 300_000; | ||
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
|
||
|
||
// ============ Constructor ============ | ||
|
||
|
@@ -45,10 +51,10 @@ | |
uint32 _destinationDomain, | ||
bytes32 _ism, | ||
address _l2Messenger, | ||
uint32 _gasQuote | ||
address _childHook | ||
|
||
) AbstractMessageIdAuthHook(_mailbox, _destinationDomain, _ism) { | ||
GAS_QUOTE = _gasQuote; | ||
l2Messenger = ICrossDomainMessenger(_l2Messenger); | ||
childHook = AbstractPostDispatchHook(_childHook); | ||
} | ||
|
||
/// @inheritdoc IPostDispatchHook | ||
|
@@ -58,27 +64,32 @@ | |
|
||
/// @inheritdoc AbstractPostDispatchHook | ||
function _quoteDispatch( | ||
bytes calldata, | ||
bytes calldata | ||
bytes calldata metadata, | ||
bytes calldata message | ||
) internal view override returns (uint256) { | ||
return GAS_QUOTE; | ||
return | ||
metadata.msgValue(0) + childHook.quoteDispatch(metadata, message); | ||
} | ||
|
||
// ============ Internal functions ============ | ||
|
||
/// @inheritdoc AbstractMessageIdAuthHook | ||
function _sendMessageId( | ||
bytes calldata metadata, | ||
bytes memory payload | ||
bytes calldata message | ||
) internal override { | ||
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
|
||
AbstractMessageIdAuthorizedIsm.verifyMessageId, | ||
message.id() | ||
); | ||
|
||
childHook.postDispatch{ | ||
value: childHook.quoteDispatch(metadata, message) | ||
|
||
}(metadata, message); | ||
aroralanuk marked this conversation as resolved.
Show resolved
Hide resolved
|
||
l2Messenger.sendMessage{value: metadata.msgValue(0)}( | ||
TypeCasts.bytes32ToAddress(ism), | ||
payload, | ||
GAS_QUOTE | ||
MIN_GAS_LIMIT | ||
); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,7 @@ | |
import {StandardHookMetadata} from "./libs/StandardHookMetadata.sol"; | ||
import {TypeCasts} from "../libs/TypeCasts.sol"; | ||
import {Message} from "../libs/Message.sol"; | ||
import {AbstractMessageIdAuthorizedIsm} from "../isms/hook/AbstractMessageIdAuthorizedIsm.sol"; | ||
import {IPostDispatchHook} from "../interfaces/hooks/IPostDispatchHook.sol"; | ||
|
||
// ============ External Imports ============ | ||
|
@@ -32,6 +33,7 @@ | |
*/ | ||
contract OPStackHook is AbstractMessageIdAuthHook { | ||
using StandardHookMetadata for bytes; | ||
using Message for bytes; | ||
|
||
// ============ Constants ============ | ||
|
||
|
@@ -60,21 +62,22 @@ | |
|
||
// ============ Internal functions ============ | ||
function _quoteDispatch( | ||
bytes calldata, | ||
bytes calldata metadata, | ||
bytes calldata | ||
) internal pure override returns (uint256) { | ||
return 0; // gas subsidized by the L2 | ||
return metadata.msgValue(0); // gas subsidized by the L2 | ||
} | ||
|
||
/// @inheritdoc AbstractMessageIdAuthHook | ||
function _sendMessageId( | ||
bytes calldata metadata, | ||
bytes memory payload | ||
bytes calldata message | ||
) internal override { | ||
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
|
||
AbstractMessageIdAuthorizedIsm.verifyMessageId, | ||
message.id() | ||
); | ||
|
||
l1Messenger.sendMessage{value: metadata.msgValue(0)}( | ||
TypeCasts.bytes32ToAddress(ism), | ||
payload, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,7 @@ | |
import {TypeCasts} from "../libs/TypeCasts.sol"; | ||
import {Message} from "../libs/Message.sol"; | ||
import {IPostDispatchHook} from "../interfaces/hooks/IPostDispatchHook.sol"; | ||
import {AbstractMessageIdAuthorizedIsm} from "../isms/hook/AbstractMessageIdAuthorizedIsm.sol"; | ||
|
||
// ============ External Imports ============ | ||
import {FxBaseRootTunnel} from "fx-portal/contracts/tunnel/FxBaseRootTunnel.sol"; | ||
|
@@ -31,6 +32,7 @@ | |
*/ | ||
contract PolygonPosHook is AbstractMessageIdAuthHook, FxBaseRootTunnel { | ||
using StandardHookMetadata for bytes; | ||
using Message for bytes; | ||
|
||
// ============ Constructor ============ | ||
|
||
|
@@ -56,22 +58,27 @@ | |
|
||
// ============ Internal functions ============ | ||
function _quoteDispatch( | ||
bytes calldata, | ||
bytes calldata metadata, | ||
bytes calldata | ||
) internal pure override returns (uint256) { | ||
return 0; | ||
return metadata.msgValue(0); | ||
} | ||
|
||
/// @inheritdoc AbstractMessageIdAuthHook | ||
function _sendMessageId( | ||
bytes calldata metadata, | ||
bytes memory payload | ||
bytes calldata message | ||
) internal override { | ||
require( | ||
metadata.msgValue(0) == 0, | ||
"PolygonPosHook: does not support msgValue" | ||
); | ||
require(msg.value == 0, "PolygonPosHook: does not support msgValue"); | ||
|
||
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
|
||
AbstractMessageIdAuthorizedIsm.verifyMessageId, | ||
message.id() | ||
); | ||
_sendMessageToChild(payload); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,9 +15,11 @@ | |
|
||
// ============ Internal Imports ============ | ||
import {TypeCasts} from "../../libs/TypeCasts.sol"; | ||
import {Message} from "../../libs/Message.sol"; | ||
import {IPostDispatchHook} from "../../interfaces/hooks/IPostDispatchHook.sol"; | ||
import {IMessageDispatcher} from "../../interfaces/hooks/IMessageDispatcher.sol"; | ||
import {AbstractMessageIdAuthHook} from "../libs/AbstractMessageIdAuthHook.sol"; | ||
import {AbstractMessageIdAuthorizedIsm} from "../../isms/hook/AbstractMessageIdAuthorizedIsm.sol"; | ||
|
||
// ============ External Imports ============ | ||
import {Address} from "@openzeppelin/contracts/utils/Address.sol"; | ||
|
@@ -28,6 +30,8 @@ | |
* any of the 5164 adapters. | ||
*/ | ||
contract ERC5164Hook is AbstractMessageIdAuthHook { | ||
using Message for bytes; | ||
|
||
IMessageDispatcher public immutable dispatcher; | ||
|
||
constructor( | ||
|
@@ -55,9 +59,14 @@ | |
function _sendMessageId( | ||
bytes calldata, | ||
/* metadata */ | ||
bytes memory payload | ||
bytes calldata message | ||
) internal override { | ||
require(msg.value == 0, "ERC5164Hook: no value allowed"); | ||
|
||
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
|
||
AbstractMessageIdAuthorizedIsm.verifyMessageId, | ||
message.id() | ||
); | ||
dispatcher.dispatchMessage( | ||
destinationDomain, | ||
TypeCasts.bytes32ToAddress(ism), | ||
|
aroralanuk marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,7 @@ | |
import {Indexed} from "../../libs/Indexed.sol"; | ||
import {IPostDispatchHook} from "../../interfaces/hooks/IPostDispatchHook.sol"; | ||
import {AbstractMessageIdAuthHook} from "../libs/AbstractMessageIdAuthHook.sol"; | ||
import {AbstractMessageIdAuthorizedIsm} from "../../isms/hook/AbstractMessageIdAuthorizedIsm.sol"; | ||
import {StandardHookMetadata} from "../libs/StandardHookMetadata.sol"; | ||
|
||
struct LayerZeroV2Metadata { | ||
|
@@ -55,8 +56,13 @@ | |
/// @inheritdoc AbstractMessageIdAuthHook | ||
function _sendMessageId( | ||
bytes calldata metadata, | ||
bytes memory payload | ||
bytes calldata message | ||
) internal override { | ||
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
|
||
AbstractMessageIdAuthorizedIsm.verifyMessageId, | ||
message.id() | ||
); | ||
|
||
bytes calldata lZMetadata = metadata.getCustomMetadata(); | ||
( | ||
uint32 eid, | ||
|
@@ -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: 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
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
|
||
} | ||
|
||
/// @dev payInZRO is hardcoded to false because zro tokens should not be directly accepted | ||
|
@@ -96,7 +104,7 @@ | |
message.senderAddress() | ||
); | ||
|
||
return msgFee.nativeFee; | ||
return metadata.msgValue(0) + msgFee.nativeFee; | ||
} | ||
|
||
/** | ||
|
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