Skip to content

Commit 07bfbe0

Browse files
PeterMPhillipsPeterMPhillips
authored
Merge pull request #51 from EnsoBuild/layerzero-chainsec
LayerZeroReceiver Chainsec audit issues
2 parents 4004719 + 6fb165e commit 07bfbe0

File tree

4 files changed

+45
-66
lines changed

4 files changed

+45
-66
lines changed

‎script/EnsoReceiverDeployer.s.sol‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,7 @@ contract EnsoReceiverDeployer is Script {
1212
address OWNER = 0x826e0BB2276271eFdF2a500597f37b94f6c153bA;
1313
address BACKEND_SIGNER = 0xFE503EE14863F6aCEE10BCdc66aC5e2301b3A946;
1414

15-
function run()
16-
public
17-
returns (EnsoReceiver implementation, ERC4337CloneFactory factory)
18-
{
15+
function run() public returns (EnsoReceiver implementation, ERC4337CloneFactory factory) {
1916
uint256 deployerPrivateKey = vm.envUint("PRIVATE_KEY");
2017

2118
vm.startBroadcast(deployerPrivateKey);

‎script/LayerZeroDeployer.s.sol‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ contract LayerZeroDeployer is Script {
4545
router = 0xF75584eF6673aD213a685a1B58Cc0330B8eA22Cf;
4646
}
4747

48-
lzReceiver = address(new LayerZeroReceiver{ salt: "LayerZeroReceiver" }(endpoint, router, deployer, 100_000));
48+
lzReceiver = address(new LayerZeroReceiver{ salt: "LayerZeroReceiver" }(endpoint, router, deployer));
4949

5050
vm.stopBroadcast();
5151
}

‎src/bridge/LayerZeroReceiver.sol‎

Lines changed: 25 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -13,43 +13,40 @@ contract LayerZeroReceiver is Ownable, ILayerZeroComposer {
1313
using SafeERC20 for IERC20;
1414

1515
address private constant _NATIVE_ASSET = address(0);
16-
uint256 private constant _TRY_CATCH_GAS = 2000;
1716

1817
address public immutable endpoint;
1918
IEnsoRouter public immutable router;
2019

21-
uint256 public reserveGas;
22-
2320
mapping(address => bool) public validOFT;
2421
mapping(address => bool) public validRegistrar;
22+
mapping(bytes32 => bool) public messageExecuted;
2523

2624
event ShortcutExecutionSuccessful(bytes32 guid);
2725
event ShortcutExecutionFailed(bytes32 guid, bytes error);
2826
event OFTAdded(address oft);
2927
event OFTRemoved(address oft);
3028
event RegistrarAdded(address account);
3129
event RegistrarRemoved(address account);
32-
event ReserveGasUpdated(uint256 amount);
3330
event FundsCollected(address token, uint256 amount);
3431

35-
error InsufficientGas(bytes32 guid);
32+
error InsufficientGas(bytes32 guid, uint256 estimatedGas, uint256 availableGas);
3633
error NotEndpoint(address sender);
3734
error NotRegistrar(address sender);
3835
error NotSelf();
3936
error TransferFailed(address receiver);
4037
error InvalidOFT(address oft);
4138
error EndpointNotSet();
4239
error RouterNotSet();
43-
error InvalidArrayLength();
40+
error InvalidMsgValue(uint256 actual, uint256 expected);
41+
error MessageExecuted(bytes32 key);
4442

45-
constructor(address _endpoint, address _router, address _owner, uint256 _reserveGas) Ownable(_owner) {
43+
constructor(address _endpoint, address _router, address _owner) Ownable(_owner) {
4644
if (_endpoint == address(0)) revert EndpointNotSet();
4745
if (_router == address(0)) revert RouterNotSet();
4846
endpoint = _endpoint;
4947
router = IEnsoRouter(_router);
5048
validRegistrar[_owner] = true;
51-
reserveGas = _reserveGas;
52-
emit ReserveGasUpdated(_reserveGas);
49+
emit RegistrarAdded(_owner);
5350
}
5451

5552
// layer zero callback
@@ -65,24 +62,23 @@ contract LayerZeroReceiver is Ownable, ILayerZeroComposer {
6562
{
6663
if (msg.sender != endpoint) revert NotEndpoint(msg.sender);
6764
if (!validOFT[_from]) revert InvalidOFT(_from);
65+
bytes32 key = getMessageKey(_from, _guid, _message);
66+
if (messageExecuted[key]) revert MessageExecuted(key);
6867

6968
address token = IPool(_from).token();
7069

7170
uint256 amount = _message.amountLD();
7271
bytes memory composeMsg = _message.composeMsg();
73-
(address receiver, bytes memory shortcutData) = abi.decode(composeMsg, (address, bytes));
74-
72+
(address receiver, uint256 nativeDrop, uint256 estimatedGas, bytes memory shortcutData) =
73+
abi.decode(composeMsg, (address, uint256, uint256, bytes));
74+
if (msg.value < nativeDrop) revert InvalidMsgValue(msg.value, nativeDrop);
7575
uint256 availableGas = gasleft();
76-
if (availableGas < reserveGas) revert InsufficientGas(_guid);
76+
if (availableGas < estimatedGas) revert InsufficientGas(_guid, estimatedGas, availableGas);
77+
7778
// try to execute shortcut
78-
try this.execute{ gas: availableGas - reserveGas }(token, amount, shortcutData, msg.value) {
79+
try this.execute(token, amount, shortcutData, msg.value) {
7980
emit ShortcutExecutionSuccessful(_guid);
8081
} catch (bytes memory err) {
81-
if (err.length == 0 && gasleft() < reserveGas - _TRY_CATCH_GAS) {
82-
// assume that the shortcut failed due to an out of gas error,
83-
// to discourage griefing we will revert instead of transferring funds.
84-
revert InsufficientGas(_guid);
85-
}
8682
// if shortcut fails send funds to receiver
8783
emit ShortcutExecutionFailed(_guid, err);
8884
_transfer(token, receiver, amount);
@@ -145,19 +141,19 @@ contract LayerZeroReceiver is Ownable, ILayerZeroComposer {
145141
emit RegistrarRemoved(account);
146142
}
147143

148-
function setReserveGas(uint256 amount) external onlyOwner {
149-
reserveGas = amount;
150-
emit ReserveGasUpdated(amount);
144+
// sweep funds to the contract owner in order to refund user
145+
function sweep(bytes32 messageKey, address token, uint256 amount) external onlyOwner {
146+
// message key is passed to block subsequent calls to lzCompose in case a failing message becomes executable.
147+
// internal message data is not validated in case the message itself is malformed or incorrect
148+
// (e.g. oft returns incorrect token)
149+
messageExecuted[messageKey] = true;
150+
151+
_transfer(token, owner(), amount);
152+
emit FundsCollected(token, amount);
151153
}
152154

153-
// sweep funds to the contract owner in order to refund user
154-
function sweep(address[] memory tokens, uint256[] memory amounts) external onlyOwner {
155-
if (tokens.length != amounts.length) revert InvalidArrayLength();
156-
address receiver = owner();
157-
for (uint256 i = 0; i < tokens.length; ++i) {
158-
_transfer(tokens[i], receiver, amounts[i]);
159-
emit FundsCollected(tokens[i], amounts[i]);
160-
}
155+
function getMessageKey(address from, bytes32 guid, bytes calldata message) public pure returns (bytes32) {
156+
return keccak256(abi.encode(from, guid, message));
161157
}
162158

163159
function _transfer(address token, address receiver, uint256 amount) internal {

‎test/Bridge.t.sol‎

Lines changed: 18 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ contract BridgeTest is Test {
4646
vm.selectFork(_ethereumFork);
4747
router = new EnsoRouter();
4848
shortcuts = EnsoShortcuts(payable(router.shortcuts()));
49-
lzReceiver = new LayerZeroReceiver(address(this), address(router), address(this), 100_000);
49+
lzReceiver = new LayerZeroReceiver(address(this), address(router), address(this));
5050

5151
address[] memory ofts = new address[](2);
5252
ofts[0] = ethPool;
@@ -60,7 +60,7 @@ contract BridgeTest is Test {
6060
uint256 balanceBefore = weth.balanceOf(address(this));
6161

6262
(bytes32[] memory commands, bytes[] memory state) = _buildWethDeposit(ETH_AMOUNT);
63-
bytes memory message = _buildLzComposeMessage(ETH_AMOUNT, commands, state);
63+
bytes memory message = _buildLzComposeMessage(ETH_AMOUNT, 0, 0, commands, state);
6464

6565
// transfer funds
6666
(bool success,) = address(lzReceiver).call{ value: ETH_AMOUNT }("");
@@ -78,7 +78,7 @@ contract BridgeTest is Test {
7878

7979
// TOO MUCH VALUE ATTEMPTED TO TRANSFER
8080
(bytes32[] memory commands, bytes[] memory state) = _buildWethDeposit(ETH_AMOUNT * 100);
81-
bytes memory message = _buildLzComposeMessage(ETH_AMOUNT, commands, state);
81+
bytes memory message = _buildLzComposeMessage(ETH_AMOUNT, 0, 0, commands, state);
8282

8383
// transfer funds
8484
(bool success,) = address(lzReceiver).call{ value: ETH_AMOUNT }("");
@@ -91,32 +91,21 @@ contract BridgeTest is Test {
9191
assertEq(balanceBefore, address(this).balance);
9292
}
9393

94-
function testEthBridgeWithShortcutOutOfGas() public {
94+
function testEthBridgeWithLessThanEstimateGas() public {
9595
vm.selectFork(_ethereumFork);
9696

9797
(bytes32[] memory commands, bytes[] memory state) = _buildWethDeposit(ETH_AMOUNT);
98-
bytes memory message = _buildLzComposeMessage(ETH_AMOUNT, commands, state);
98+
// exact gas amount needed for execution
99+
uint256 estimatedGas = 75_272;
100+
bytes memory message = _buildLzComposeMessage(ETH_AMOUNT, 0, estimatedGas, commands, state);
99101

100102
// transfer funds
101103
(bool success,) = address(lzReceiver).call{ value: ETH_AMOUNT }("");
102104
if (!success) revert TransferFailed();
103105
// trigger compose with insufficient gas
104-
vm.expectRevert();
105-
lzReceiver.lzCompose{ gas: 108_000 }(ethPool, bytes32(0), message, address(0), "");
106-
}
107-
108-
function testEthBridgeWithLessThanReserveGas() public {
109-
vm.selectFork(_ethereumFork);
110-
111-
(bytes32[] memory commands, bytes[] memory state) = _buildWethDeposit(ETH_AMOUNT);
112-
bytes memory message = _buildLzComposeMessage(ETH_AMOUNT, commands, state);
113-
114-
// transfer funds
115-
(bool success,) = address(lzReceiver).call{ value: ETH_AMOUNT }("");
116-
if (!success) revert TransferFailed();
117-
// trigger compose with insufficient gas
118-
vm.expectRevert();
119-
lzReceiver.lzCompose{ gas: 99_000 }(ethPool, bytes32(0), message, address(0), "");
106+
vm.expectRevert(abi.encodeWithSelector(LayerZeroReceiver.InsufficientGas.selector, bytes32(0), estimatedGas, estimatedGas - 1));
107+
// exactly 1 less gas than needed for lz compose
108+
lzReceiver.lzCompose{ gas: 85_663 }(ethPool, bytes32(0), message, address(0), "");
120109
}
121110

122111
function testUsdcBridge() public {
@@ -125,7 +114,7 @@ contract BridgeTest is Test {
125114
uint256 balanceBefore = IERC20(usdc).balanceOf(vitalik);
126115

127116
(bytes32[] memory commands, bytes[] memory state) = _buildTransfer(usdc, vitalik, USDC_AMOUNT);
128-
bytes memory message = _buildLzComposeMessage(USDC_AMOUNT, commands, state);
117+
bytes memory message = _buildLzComposeMessage(USDC_AMOUNT, 0, 0, commands, state);
129118

130119
// transfer funds
131120
vm.startPrank(usdcPool);
@@ -145,7 +134,7 @@ contract BridgeTest is Test {
145134

146135
(bytes32[] memory commands, bytes[] memory state) =
147136
_buildTokenAndValueTransfer(usdc, vitalik, USDC_AMOUNT, ETH_AMOUNT);
148-
bytes memory message = _buildLzComposeMessage(USDC_AMOUNT, commands, state);
137+
bytes memory message = _buildLzComposeMessage(USDC_AMOUNT, ETH_AMOUNT, 0, commands, state);
149138

150139
// transfer funds
151140
vm.startPrank(usdcPool);
@@ -168,7 +157,7 @@ contract BridgeTest is Test {
168157

169158
// TOO MUCH VALUE ATTEMPTED TO TRANSFER
170159
(bytes32[] memory commands, bytes[] memory state) = _buildTransfer(usdc, vitalik, USDC_AMOUNT * 100);
171-
bytes memory message = _buildLzComposeMessage(USDC_AMOUNT, commands, state);
160+
bytes memory message = _buildLzComposeMessage(USDC_AMOUNT, 0, 0, commands, state);
172161

173162
// transfer funds
174163
vm.startPrank(usdcPool);
@@ -201,13 +190,8 @@ contract BridgeTest is Test {
201190
uint256 wethBalanceBefore = weth.balanceOf(address(this));
202191

203192
// sweep
204-
address[] memory tokens = new address[](2);
205-
tokens[0] = eth;
206-
tokens[1] = address(weth);
207-
uint256[] memory amounts = new uint256[](2);
208-
amounts[0] = address(lzReceiver).balance;
209-
amounts[1] = weth.balanceOf(address(lzReceiver));
210-
lzReceiver.sweep(tokens, amounts);
193+
lzReceiver.sweep(bytes32(uint256(1)), eth, address(lzReceiver).balance);
194+
lzReceiver.sweep(bytes32(uint256(2)), address(weth), weth.balanceOf(address(lzReceiver)));
211195

212196
uint256 ethBalanceAfter = address(this).balance;
213197
uint256 wethBalanceAfter = weth.balanceOf(address(this));
@@ -219,6 +203,8 @@ contract BridgeTest is Test {
219203

220204
function _buildLzComposeMessage(
221205
uint256 amount,
206+
uint256 nativeDrop,
207+
uint256 estimatedGas,
222208
bytes32[] memory commands,
223209
bytes[] memory state
224210
)
@@ -230,7 +216,7 @@ contract BridgeTest is Test {
230216
bytes memory shortcutData =
231217
abi.encodeWithSelector(shortcuts.executeShortcut.selector, bytes32(0), bytes32(0), commands, state);
232218
// encode callback data
233-
bytes memory callbackData = abi.encode(address(this), shortcutData);
219+
bytes memory callbackData = abi.encode(address(this), nativeDrop, estimatedGas, shortcutData);
234220
// encode message
235221
message = OFTComposeMsgCodec.encode(
236222
uint64(0),

0 commit comments

Comments
 (0)