Skip to content

Commit

Permalink
Merge branch 'send-lost-gas-to-0' into ackee-audit
Browse files Browse the repository at this point in the history
  • Loading branch information
reednaa committed May 3, 2024
2 parents d50ca3a + 2fbcf02 commit bde53df
Show file tree
Hide file tree
Showing 9 changed files with 38 additions and 29 deletions.
4 changes: 3 additions & 1 deletion src/IncentivizedMessageEscrow.sol
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,10 @@ abstract contract IncentivizedMessageEscrow is IIncentivizedMessageEscrow, Bytes

/**
* @param sendLostGasTo It should only be set to an EOA or a contract which implements either a fallback or a receive function that never reverts.
* It can be set to address 0 or a similar burn address if no-one wants to take ownership of the ether.
* It cannot be set to address 0, instead use a burn address (0xdead) if no-one wants to take responsibility of the Ether.
*/
constructor(address sendLostGasTo) {
if (sendLostGasTo == address(0)) revert SendLostGasToIsZero();
SEND_LOST_GAS_TO = sendLostGasTo;
}

Expand Down Expand Up @@ -351,6 +352,7 @@ abstract contract IncentivizedMessageEscrow is IIncentivizedMessageEscrow, Bytes
bytes32 feeRecipient
) external virtual payable {
uint256 gasLimit = gasleft(); // uint256 is used here instead of uint48, since there is no advantage to uint48 until after we calculate the difference.
if (feeRecipient == bytes32(0)) revert FeeRecipientIsZero();

// Verify that the message is authentic and remove potential context that the messaging protocol added to the message.
(bytes32 chainIdentifier, bytes memory implementationIdentifier, bytes calldata message) = _verifyPacket(messagingProtocolContext, rawMessage);
Expand Down
42 changes: 22 additions & 20 deletions src/interfaces/IMessageEscrowErrors.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,26 @@
pragma solidity >=0.8.0;

interface IMessageEscrowErrors {
error NotEnoughGasProvided(uint128 expected, uint128 actual); // 030748b5
error MessageAlreadyBountied(); // 068a62ee
error MessageDoesNotExist(); // 970e41ec
error MessageAlreadyAcked(); // 8af35858
error NotImplementedError(); // d41c17e7
error MessageAlreadySpent(); // e954aba2
error AckHasNotBeenExecuted(); // 3d1553f8
error NoImplementationAddressSet(); // 9f994b4b
error InvalidImplementationAddress(); // c970156c
error IncorrectValueProvided(uint128 expected, uint128 actual); // 0b52a60b
error ImplementationAddressAlreadySet(bytes currentImplementation); // dba47850
error NotEnoughGasExecution(); // 6bc33587
error RefundGasToIsZero(); // 6a1a6afe
error DeadlineTooLong(uint64 maxAllowed, uint64 actual); // 54090af9
error DeadlineInPast(uint64 blocktimestamp, uint64 actual); // 2d098d59
error CannotRetryWrongMessage(bytes32 expected, bytes32 actual); // 48ce7fac
error MessageAlreadyProcessed(); // 7b042609
error DeadlineNotPassed(uint64 expected, uint64 actual); // 862c57f4
error InvalidTimeoutPackage(bytes32 expected, bytes32 actual); // e020885d
error MessageHasInvalidContext(); // 3fcdbaba
error AckHasNotBeenExecuted(); // 0x3d1553f8
error CannotRetryWrongMessage(bytes32,bytes32); // 0x48ce7fac
error DeadlineInPast(uint64 blocktimestamp, uint64 actual); // 0x2d098d59
error DeadlineNotPassed(uint64 expected, uint64 actual); // 0x862c57f4
error DeadlineTooLong(uint64 maxAllowed, uint64 actual); // 0x3c06f369
error ImplementationAddressAlreadySet(bytes currentImplementation); // 0xdba47850
error IncorrectValueProvided(uint128 expected, uint128 actual); // 0x0b52a60b
error InvalidImplementationAddress(); // 0xc970156c
error InvalidTimeoutPackage(bytes32 expected, bytes32 actual); // 0xe020885d
error MessageAlreadyAcked(); // 0x8af35858
error MessageAlreadyBountied(); // 0x068a62ee
error MessageAlreadyProcessed(); // 0x7b042609
error MessageAlreadySpent(); // 0xe954aba2
error MessageDoesNotExist(); // 0x970e41ec
error MessageHasInvalidContext(); // 0x3fcdbaba
error NoImplementationAddressSet(); // 0x9f994b4b
error NotEnoughGasExecution(); // 0x6bc33587
error NotEnoughGasProvided(uint128 expected, uint128 actual); // 0x030748b5
error NotImplementedError(); // 0xd41c17e7
error RefundGasToIsZero(); // 0x6a1a6afe
error SendLostGasToIsZero(); // 0x470337f2
error FeeRecipientIsZero(); // 0x6a1a6afe
}
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ contract MessageIdentifierTest is TestCommon {
vm.assume(a != b);
vm.assume(a != address(application));
vm.assume(b != address(application));
vm.assume(a != address(this));
vm.assume(b != address(this));
IncentiveDescription storage incentive = _INCENTIVE;

vm.deal(a, _getTotalIncentive(_INCENTIVE));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ contract sendPacketPaymentTest is TestCommon {

uint128 constant SEND_MESSAGE_PAYMENT_COST = 10_000;

address constant SEND_LOST_GAS_TO = address(0xdead);

uint256 _receive;

event Message(
Expand All @@ -26,7 +28,7 @@ contract sendPacketPaymentTest is TestCommon {
(SIGNER, PRIVATEKEY) = makeAddrAndKey("signer");
_REFUND_GAS_TO = makeAddr("Alice");
BOB = makeAddr("Bob");
escrow = new IncentivizedMockEscrow(sendLostGasTo, _DESTINATION_IDENTIFIER, SIGNER, SEND_MESSAGE_PAYMENT_COST, 0);
escrow = new IncentivizedMockEscrow(SEND_LOST_GAS_TO, _DESTINATION_IDENTIFIER, SIGNER, SEND_MESSAGE_PAYMENT_COST, 0);

application = ICrossChainReceiver(address(new MockApplication(address(escrow))));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ contract GasSpendControlTest is TestCommon {
messageIdentifier,
_DESTINATION_ADDRESS_APPLICATION,
destinationFeeRecipient,
uint48(0x3697d), // Gas used
uint48(0x36996), // Gas used
uint64(1),
bytes1(0xff), // This states that the call went wrong.
message
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ contract processPacketNoReceiveTest is TestCommon {
messageIdentifier,
_DESTINATION_ADDRESS_THIS,
feeRecipient,
uint48(0x8350), // Gas used
uint48(0x8369), // Gas used
uint64(1),
abi.encodePacked(bytes1(0xff)),
message
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ contract processPacketCallTest is TestCommon {
messageIdentifier,
_DESTINATION_ADDRESS_APPLICATION,
feeRecipient,
uint48(0x7d66), // Gas used
uint48(0x7d7f), // Gas used
uint64(1),
mockAck
)
Expand Down Expand Up @@ -92,6 +92,7 @@ contract processPacketCallTest is TestCommon {
}

function test_expect_caller(address caller) public {
vm.assume(caller != address(this));
vm.assume(caller != 0x2e234DAe75C793f67A35089C9d99245E1C58470b);
bytes memory message = _MESSAGE;
bytes32 feeRecipient = bytes32(uint256(uint160(address(this))));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ contract ReemitAckAnywhereExploitTest is TestCommon {
// lets execute the non-legit message

vm.recordLogs();
escrow.processPacket(nonLegitMessageContext, nonLegitMessage, bytes32(abi.encode(address(0))));
escrow.processPacket(nonLegitMessageContext, nonLegitMessage, bytes32(abi.encode(address(0xdead))));
Vm.Log[] memory entries = vm.getRecordedLogs();

(, , bytes memory ackMessage) = abi.decode(entries[1].data, (bytes32, bytes, bytes));
Expand Down
6 changes: 3 additions & 3 deletions test/TestCommon.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ interface ICansubmitMessage is IMessageEscrowStructs{

contract TestCommon is Test, IMessageEscrowEvents, IMessageEscrowStructs {

uint256 constant GAS_SPENT_ON_SOURCE = 6899;
uint256 constant GAS_SPENT_ON_DESTINATION = 32102;
uint256 constant GAS_SPENT_ON_SOURCE = 6642;
uint256 constant GAS_SPENT_ON_DESTINATION = 32127;

bytes32 constant _DESTINATION_IDENTIFIER = bytes32(uint256(0x123123) + uint256(2**255));

Expand All @@ -36,7 +36,7 @@ contract TestCommon is Test, IMessageEscrowEvents, IMessageEscrowStructs {
bytes _DESTINATION_ADDRESS_APPLICATION;

address SIGNER;
address sendLostGasTo;
address sendLostGasTo = address(0xdead);
address BOB;
uint256 PRIVATEKEY;

Expand Down

0 comments on commit bde53df

Please sign in to comment.