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 audit issue #61

Merged
merged 3 commits into from
Oct 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 0 additions & 38 deletions src/ORMP.sol
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,6 @@ import "./security/ExcessivelySafeCall.sol";
contract ORMP is ReentrancyGuard, Channel {
using ExcessivelySafeCall for address;

/// msgHash => isFailed
mapping(bytes32 => bool) public fails;

/// @dev Notifies an observer that the failed message has been cleared.
/// @param msgHash Hash of the message.
event ClearFailedMessage(bytes32 indexed msgHash);
/// @dev Notifies an observer that the failed message has been retried.
/// @param msgHash Hash of the message.
/// @param dispatchResult Result of the message dispatch.
event RetryFailedMessage(bytes32 indexed msgHash, bool dispatchResult);

constructor(address dao) Channel(dao) {}

/// @dev Send a cross-chain message over the endpoint.
Expand Down Expand Up @@ -126,37 +115,10 @@ contract ORMP is ReentrancyGuard, Channel {
{
bytes32 msgHash = _recv(message, proof);
dispatchResult = _dispatch(message, msgHash, gasLimit);
if (!dispatchResult) {
fails[msgHash] = true;
}
// emit dispatched message event.
emit MessageDispatched(msgHash, dispatchResult);
}

/// @dev Retry failed message.
/// @param message Failed message info.
/// @return dispatchResult Result of the message dispatch.
function retryFailedMessage(Message calldata message) external recvNonReentrant returns (bool dispatchResult) {
bytes32 msgHash = hash(message);
require(fails[msgHash] == true, "!failed");
dispatchResult = _dispatch(message, msgHash, gasleft());
if (dispatchResult) {
delete fails[msgHash];
}
emit RetryFailedMessage(msgHash, dispatchResult);
}

/// @dev Retry failed message.
/// @notice Only message.to could clear this message.
/// @param message Failed message info.
function clearFailedMessage(Message calldata message) external {
bytes32 msgHash = hash(message);
require(fails[msgHash] == true, "!failed");
require(message.to == msg.sender, "!auth");
delete fails[msgHash];
emit ClearFailedMessage(msgHash);
}

/// @dev Dispatch the cross chain message.
function _dispatch(Message memory message, bytes32 msgHash, uint256 gasLimit)
private
Expand Down
5 changes: 1 addition & 4 deletions src/Verifier.sol
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,7 @@ abstract contract Verifier is IVerifier {
bytes32[32] messageProof;
}

/// @notice Fetch message root oracle.
/// @param chainId The destination chain id.
/// @param blockNumber The block number where the message root is located.
/// @return Message root in destination chain.
/// @inheritdoc IVerifier
function merkleRoot(uint256 chainId, uint256 blockNumber) public view virtual returns (bytes32);

/// @inheritdoc IVerifier
Expand Down
10 changes: 0 additions & 10 deletions src/interfaces/IEndpoint.sol
Original file line number Diff line number Diff line change
Expand Up @@ -42,16 +42,6 @@ interface IEndpoint {
view
returns (uint256);

/// @dev Retry failed message.
/// @notice Only message.to could clear this message.
/// @param message Failed message info.
function clearFailedMessage(Message calldata message) external;

/// @dev Retry failed message.
/// @param message Failed message info.
/// @return dispatchResult Result of the message dispatch.
function retryFailedMessage(Message calldata message) external returns (bool dispatchResult);

/// @dev Recv verified message and dispatch to destination user application address.
/// @param message Verified receive message info.
/// @param proof Message proof of this message.
Expand Down
6 changes: 6 additions & 0 deletions src/interfaces/IVerifier.sol
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@
pragma solidity 0.8.17;

interface IVerifier {
/// @notice Fetch message root oracle.
/// @param chainId The destination chain id.
/// @param blockNumber The block number where the message root is located.
/// @return Message root in destination chain.
function merkleRoot(uint256 chainId, uint256 blockNumber) external view returns (bytes32);

/// @notice Verify message proof
/// @dev Message proof provided by relayer. Oracle should provide message root of
/// source chain, and verify the merkle proof of the message hash.
Expand Down
17 changes: 5 additions & 12 deletions src/user/Application.sol
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,9 @@ abstract contract Application {
IEndpoint(TRUSTED_ORMP).setAppConfig(oracle, relayer);
}

function _clearFailedMessage(Message memory message) internal virtual {
return IEndpoint(TRUSTED_ORMP).clearFailedMessage(message);
}

function retryFailedMessage(Message calldata message) public virtual returns (bool dispatchResult) {
return IEndpoint(TRUSTED_ORMP).retryFailedMessage(message);
}

function isTrustedORMP(address ormp) public view returns (bool) {
return TRUSTED_ORMP == ormp;
modifier onlyORMP() {
require(TRUSTED_ORMP == msg.sender, "!ormp");
_;
}

function _messageId() internal pure returns (bytes32 _msgDataMessageId) {
Expand All @@ -58,8 +51,8 @@ abstract contract Application {
}
}

function _xmsgSender() internal view returns (address payable _from) {
require(msg.data.length >= 20 && isTrustedORMP(msg.sender), "!xmsgSender");
function _xmsgSender() internal pure returns (address payable _from) {
require(msg.data.length >= 20, "!xmsgSender");
assembly {
_from := shr(96, calldataload(sub(calldatasize(), 20)))
}
Expand Down
21 changes: 0 additions & 21 deletions test/ORMP.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -52,27 +52,6 @@ contract ORMPTest is Test, Verifier {
assertEq(r, false);
}

function test_retry() public {
perform_send();
bool r = ormp.recv(message, abi.encode(proof), gasleft());
assertEq(r, false);
r = ormp.retryFailedMessage(message);
assertEq(r, false);
}

function test_clear() public {
bytes32 msgHash = hash(message);
bool failed = ormp.fails(msgHash);
assertEq(failed, false);
perform_send();
ormp.recv(message, abi.encode(proof), gasleft());
failed = ormp.fails(msgHash);
assertEq(failed, true);
ormp.clearFailedMessage(message);
failed = ormp.fails(msgHash);
assertEq(failed, false);
}

function fee(uint256, address) external pure returns (uint256) {
return 2;
}
Expand Down
4 changes: 0 additions & 4 deletions test/user/Application.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,6 @@ contract ApplicationTest is Test {
contract UserApplication is Application {
constructor(address ormp) Application(ormp) {}

function clearFailedMessage(Message calldata message) public {}

function retryFailedMessage(Message calldata message) public override returns (bool dispatchResult) {}

function setAppConfig(address relayer, address oracle) public {}

function recv() public view {
Expand Down