Skip to content

Commit

Permalink
STABLE-7554: Internal audit changes (circlefin#44)
Browse files Browse the repository at this point in the history
  • Loading branch information
ams9198 authored Nov 5, 2024
1 parent 2e28bff commit 47dcbca
Show file tree
Hide file tree
Showing 14 changed files with 273 additions and 133 deletions.
26 changes: 11 additions & 15 deletions src/MessageTransmitter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -247,12 +247,10 @@ contract MessageTransmitter is
* of the attester address recovered from signatures.
* @return success bool, true if successful
*/
function receiveMessage(bytes calldata message, bytes calldata attestation)
external
override
whenNotPaused
returns (bool success)
{
function receiveMessage(
bytes calldata message,
bytes calldata attestation
) external override whenNotPaused returns (bool success) {
// Validate each signature in the attestation
_verifyAttestationSignatures(message, attestation);

Expand Down Expand Up @@ -313,10 +311,9 @@ contract MessageTransmitter is
* to avoid impacting users who rely on large messages.
* @param newMaxMessageBodySize new max message body size, in bytes
*/
function setMaxMessageBodySize(uint256 newMaxMessageBodySize)
external
onlyOwner
{
function setMaxMessageBodySize(
uint256 newMaxMessageBodySize
) external onlyOwner {
maxMessageBodySize = newMaxMessageBodySize;
emit MaxMessageBodySizeUpdated(maxMessageBodySize);
}
Expand Down Expand Up @@ -372,11 +369,10 @@ contract MessageTransmitter is
destination
* @return hash of source and nonce
*/
function _hashSourceAndNonce(uint32 _source, uint64 _nonce)
internal
pure
returns (bytes32)
{
function _hashSourceAndNonce(
uint32 _source,
uint64 _nonce
) internal pure returns (bytes32) {
return keccak256(abi.encodePacked(_source, _nonce));
}

Expand Down
4 changes: 2 additions & 2 deletions src/messages/v2/AddressUtils.sol
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ library AddressUtils {
* @notice Converts an address to bytes32 by left-padding with zeros (alignment preserving cast.)
* @param addr The address to convert to bytes32
*/
function addressToBytes32(address addr) internal pure returns (bytes32) {
function toBytes32(address addr) internal pure returns (bytes32) {
return bytes32(uint256(uint160(addr)));
}

Expand All @@ -36,7 +36,7 @@ library AddressUtils {
* For use cases where this is not acceptable, validate that the first 12 bytes of _buf are zero-padding.
* @param _buf the bytes32 to convert to address
*/
function bytes32ToAddress(bytes32 _buf) internal pure returns (address) {
function toAddress(bytes32 _buf) internal pure returns (address) {
return address(uint160(uint256(_buf)));
}
}
2 changes: 1 addition & 1 deletion src/messages/v2/MessageV2.sol
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ library MessageV2 {
bytes32 _recipient,
bytes32 _destinationCaller,
uint32 _minFinalityThreshold,
bytes memory _messageBody
bytes calldata _messageBody
) internal pure returns (bytes memory) {
return
abi.encodePacked(
Expand Down
7 changes: 1 addition & 6 deletions src/v2/BaseMessageTransmitter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,9 @@
*/
pragma solidity 0.7.6;

import {IMessageTransmitterV2} from "../interfaces/v2/IMessageTransmitterV2.sol";
import {Attestable} from "../roles/Attestable.sol";
import {Pausable} from "../roles/Pausable.sol";
import {Rescuable} from "../roles/Rescuable.sol";
import {MessageV2} from "../messages/v2/MessageV2.sol";
import {AddressUtils} from "../messages/v2/AddressUtils.sol";
import {TypedMemView} from "@memview-sol/contracts/TypedMemView.sol";
import {IMessageHandlerV2} from "../interfaces/v2/IMessageHandlerV2.sol";
import {Initializable} from "../proxy/Initializable.sol";

/**
Expand Down Expand Up @@ -85,7 +80,7 @@ contract BaseMessageTransmitter is
/**
* @notice Returns the current initialized version
*/
function initializedVersion() public view returns (uint64) {
function initializedVersion() external view returns (uint64) {
return _getInitializedVersion();
}
}
6 changes: 2 additions & 4 deletions src/v2/BaseTokenMessenger.sol
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ abstract contract BaseTokenMessenger is Rescuable, Denylistable, Initializable {
/**
* @notice Returns the current initialized version
*/
function initializedVersion() public view returns (uint64) {
function initializedVersion() external view returns (uint64) {
return _getInitializedVersion();
}

Expand Down Expand Up @@ -260,9 +260,7 @@ abstract contract BaseTokenMessenger is Rescuable, Denylistable, Initializable {
* @return true if message sender is the registered local message transmitter
*/
function _isLocalMessageTransmitter() internal view returns (bool) {
return
address(localMessageTransmitter) != address(0) &&
msg.sender == address(localMessageTransmitter);
return msg.sender == address(localMessageTransmitter);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/v2/FinalityThresholds.sol
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pragma solidity 0.7.6;
// The threshold at which (and above) messages are considered finalized.
uint32 constant FINALITY_THRESHOLD_FINALIZED = 2000;

// The threshold at which (and above) messages are considered finalized.
// The threshold at which (and above) messages are considered confirmed.
uint32 constant FINALITY_THRESHOLD_CONFIRMED = 1000;

// The minimum allowed level of finality accepted by TokenMessenger
Expand Down
18 changes: 12 additions & 6 deletions src/v2/MessageTransmitterV2.sol
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,12 @@ contract MessageTransmitterV2 is IMessageTransmitterV2, BaseMessageTransmitter {
);

// ============ Libraries ============
using AddressUtils for address;
using AddressUtils for address payable;
using AddressUtils for bytes32;
using MessageV2 for bytes29;
using TypedMemView for bytes;
using TypedMemView for bytes29;
using MessageV2 for bytes29;

// ============ Constructor ============
/**
Expand Down Expand Up @@ -117,9 +120,13 @@ contract MessageTransmitterV2 is IMessageTransmitterV2, BaseMessageTransmitter {
maxMessageBodySize = maxMessageBodySize_;

// Attester configuration
for (uint256 i; i < attesters_.length; i++) {
uint256 _attestersLength = attesters_.length;
for (uint256 i; i < _attestersLength; ++i) {
_enableAttester(attesters_[i]);
}

// Claim 0-nonce
usedNonces[bytes32(0)] = 1;
}

// ============ External Functions ============
Expand Down Expand Up @@ -147,7 +154,7 @@ contract MessageTransmitterV2 is IMessageTransmitterV2, BaseMessageTransmitter {
);
require(recipient != bytes32(0), "Recipient must be nonzero");

bytes32 _messageSender = AddressUtils.addressToBytes32(msg.sender);
bytes32 _messageSender = msg.sender.toBytes32();

// serialize message
bytes memory _message = MessageV2._formatMessageForRelay(
Expand Down Expand Up @@ -292,8 +299,7 @@ contract MessageTransmitterV2 is IMessageTransmitterV2, BaseMessageTransmitter {
// Validate destination caller
if (_msg._getDestinationCaller() != bytes32(0)) {
require(
_msg._getDestinationCaller() ==
AddressUtils.addressToBytes32(msg.sender),
_msg._getDestinationCaller() == msg.sender.toBytes32(),
"Invalid caller for message"
);
}
Expand All @@ -308,7 +314,7 @@ contract MessageTransmitterV2 is IMessageTransmitterV2, BaseMessageTransmitter {
// Unpack remaining values
_sourceDomain = _msg._getSourceDomain();
_sender = _msg._getSender();
_recipient = AddressUtils.bytes32ToAddress(_msg._getRecipient());
_recipient = _msg._getRecipient().toAddress();
_finalityThresholdExecuted = _msg._getFinalityThresholdExecuted();
_messageBody = _msg._getMessageBody().clone();
}
Expand Down
16 changes: 10 additions & 6 deletions src/v2/TokenMessengerV2.sol
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import {IRelayerV2} from "../interfaces/v2/IRelayerV2.sol";
import {IMessageHandlerV2} from "../interfaces/v2/IMessageHandlerV2.sol";
import {TypedMemView} from "@memview-sol/contracts/TypedMemView.sol";
import {BurnMessageV2} from "../messages/v2/BurnMessageV2.sol";
import {Initializable} from "../proxy/Initializable.sol";
import {TOKEN_MESSENGER_MIN_FINALITY_THRESHOLD} from "./FinalityThresholds.sol";

/**
Expand Down Expand Up @@ -62,9 +61,12 @@ contract TokenMessengerV2 is IMessageHandlerV2, BaseTokenMessenger {
);

// ============ Libraries ============
using AddressUtils for address;
using AddressUtils for address payable;
using AddressUtils for bytes32;
using BurnMessageV2 for bytes29;
using TypedMemView for bytes;
using TypedMemView for bytes29;
using BurnMessageV2 for bytes29;

// ============ Constructor ============
/**
Expand Down Expand Up @@ -122,7 +124,8 @@ contract TokenMessengerV2 is IMessageHandlerV2, BaseTokenMessenger {
localMinter = ITokenMinterV2(tokenMinter_);

// Remote messenger configuration
for (uint256 i; i < remoteDomains_.length; i++) {
uint256 _remoteDomainsLength = remoteDomains_.length;
for (uint256 i; i < _remoteDomainsLength; ++i) {
require(
remoteTokenMessengers_[i] != bytes32(0),
"Invalid TokenMessenger"
Expand Down Expand Up @@ -344,10 +347,10 @@ contract TokenMessengerV2 is IMessageHandlerV2, BaseTokenMessenger {
// Format message body
bytes memory _burnMessage = BurnMessageV2._formatMessageForRelay(
messageBodyVersion,
AddressUtils.addressToBytes32(_burnToken),
_burnToken.toBytes32(),
_mintRecipient,
_amount,
AddressUtils.addressToBytes32(msg.sender),
msg.sender.toBytes32(),
_maxFee,
_hookData
);
Expand Down Expand Up @@ -398,7 +401,7 @@ contract TokenMessengerV2 is IMessageHandlerV2, BaseTokenMessenger {
);

return (
AddressUtils.bytes32ToAddress(_msg._getMintRecipient()),
_msg._getMintRecipient().toAddress(),
_msg._getBurnToken(),
_msg._getAmount()
);
Expand Down Expand Up @@ -439,5 +442,6 @@ contract TokenMessengerV2 is IMessageHandlerV2, BaseTokenMessenger {
// Validate fee
_fee = _msg._getFeeExecuted();
require(_fee == 0 || _fee < _amount, "Fee equals or exceeds amount");
require(_fee <= _msg._getMaxFee(), "Fee exceeds max fee");
}
}
16 changes: 9 additions & 7 deletions test/examples/CCTPHookWrapper.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -318,14 +318,16 @@ contract CCTPHookWrapperTest is Test {
bytes memory _hookData
) internal pure returns (bytes memory) {
return
MessageV2._formatMessageForRelay(
abi.encodePacked(
_messageVersion,
0,
0,
bytes32(0),
bytes32(0),
bytes32(0),
0,
uint32(0), // sourceDomain
uint32(0), // destinationDomain
bytes32(0), // nonce
bytes32(0), // sender
bytes32(0), // recipient
bytes32(0), // destinationCaller
uint32(0), // minFinalityThreshold
uint32(0), // finalityThresholdExecuted
_createBurnMessage(_messageBodyVersion, _hookData)
);
}
Expand Down
6 changes: 3 additions & 3 deletions test/messages/v2/AddressUtils.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,13 @@ import {AddressUtils} from "../../../src/messages/v2/AddressUtils.sol";

contract AddressUtilsTest is Test {
function testAddressToBytes32Conversion(address _addr) public pure {
bytes32 _addrAsBytes32 = AddressUtils.addressToBytes32(_addr);
address _recoveredAddr = AddressUtils.bytes32ToAddress(_addrAsBytes32);
bytes32 _addrAsBytes32 = AddressUtils.toBytes32(_addr);
address _recoveredAddr = AddressUtils.toAddress(_addrAsBytes32);
assertEq(_recoveredAddr, _addr);
}

function testAddressToBytes32LeftPads(address _addr) public pure {
bytes32 _addrAsBytes32 = AddressUtils.addressToBytes32(_addr);
bytes32 _addrAsBytes32 = AddressUtils.toBytes32(_addr);

// addresses are 20 bytes, so the first 12 bytes should be 0 (left-padded)
for (uint8 i; i < 12; i++) {
Expand Down
4 changes: 2 additions & 2 deletions test/messages/v2/MessageV2.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ contract MessageV2Test is Test {
bytes32 _recipient,
bytes32 _destinationCaller,
uint32 _minFinalityThreshold,
bytes memory _messageBody
bytes calldata _messageBody
) public view {
bytes memory _message = MessageV2._formatMessageForRelay(
_version,
Expand Down Expand Up @@ -82,7 +82,7 @@ contract MessageV2Test is Test {
bytes32 _recipient,
bytes32 _destinationCaller,
uint32 _minFinalityThreshold,
bytes memory _messageBody
bytes calldata _messageBody
) public {
bytes memory _message = MessageV2._formatMessageForRelay(
_version,
Expand Down
Loading

0 comments on commit 47dcbca

Please sign in to comment.