Skip to content

Commit

Permalink
fix: forwarder security (#3)
Browse files Browse the repository at this point in the history
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **Refactor**
- Refactored the date calculation logic in the
`BokkyPooBahsDateTimeLibrary` for efficiency and readability.

- **New Features**
- Introduced remappings for paths like `@openzeppelin`, `forge-std`, and
`hardhat` for easier directory navigation.
- Refactored the `Forwarder` contract to inherit from
`ERC2771Forwarder`, simplifying contract structure.
- Updated functionality in `ERC721Whitelist` contract for Merkle proof
verification.
- Added functionality in the `CallReceiverMock` contract for testing
purposes.
- Updated access control in the `MetaDog` and `ExampleERC721` contracts
with `onlyOwner` modifiers.
- Implemented claim and withdrawal functionalities in the `CouponFacet`
contract for bonds.
- Enhanced token transfer logic in the `BondFacet` contract and added
new interfaces.
- Optimized date calculation functions in the
`BokkyPooBahsDateTimeLibrary`.

- **Documentation**
- Updated comments and variable names in the date calculation functions.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
bl0up authored Jun 27, 2024
1 parent daf26e0 commit 232633e
Show file tree
Hide file tree
Showing 12 changed files with 204 additions and 454 deletions.
92 changes: 5 additions & 87 deletions contracts/Forwarder.sol
Original file line number Diff line number Diff line change
@@ -1,92 +1,10 @@
// SPDX-License-Identifier: MIT
// SettleMint.com
// OpenZeppelin Contracts (last updated v5.0.0) (metatx/ERC2771Forwarder.sol)

pragma solidity ^0.8.24;
pragma solidity 0.8.24;

import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
import "@openzeppelin/contracts/utils/cryptography/EIP712.sol";
import "@openzeppelin/contracts/utils/introspection/ERC165.sol";
import {ERC2771Forwarder} from "@openzeppelin/contracts/metatx/ERC2771Forwarder.sol";

/**
* @dev Simple minimal forwarder to be used together with an ERC2771 compatible contract. See {ERC2771Context}.
*/
contract Forwarder is EIP712, ERC165 {
using ECDSA for bytes32;

struct ForwardRequest {
address from;
address to;
uint256 value;
uint256 gas;
uint256 nonce;
bytes data;
}

event ForwarderCreated(address indexed forwarderAddress);
event MetaTransactionExecuted(address indexed from, address indexed to);

bytes32 private constant _TYPEHASH =
keccak256(
"ForwardRequest(address from,address to,uint256 value,uint256 gas,uint256 nonce,bytes data)"
);

mapping(address => uint256) private _nonces;

constructor() EIP712("MinimalForwarder", "0.0.1") {
emit ForwarderCreated(address(this));
}

function getNonce(address from) public view returns (uint256) {
return _nonces[from];
}

function verify(
ForwardRequest calldata req,
bytes calldata signature
) public view returns (bool) {
address signer = _hashTypedDataV4(
keccak256(
abi.encode(
_TYPEHASH,
req.from,
req.to,
req.value,
req.gas,
req.nonce,
keccak256(req.data)
)
)
).recover(signature);
return _nonces[req.from] == req.nonce && signer == req.from;
}

function execute(
ForwardRequest calldata req,
bytes calldata signature
) public payable returns (bool, bytes memory) {
require(
verify(req, signature),
"MinimalForwarder: signature does not match request"
);
_nonces[req.from] = req.nonce + 1;

(bool success, bytes memory returndata) = req.to.call{
gas: req.gas,
value: req.value
}(abi.encodePacked(req.data, req.from));
// Validate that the relayer has sent enough gas for the call.
// See https://ronan.eth.link/blog/ethereum-gas-dangers/
assert(gasleft() > req.gas / 63);

emit MetaTransactionExecuted(req.from, req.to);
return (success, returndata);
}

function supportsInterface(
bytes4 interfaceId
) public view virtual override(ERC165) returns (bool) {
return
interfaceId == type(EIP712).interfaceId ||
super.supportsInterface(interfaceId);
}
contract Forwarder is ERC2771Forwarder {
constructor(string memory name) payable ERC2771Forwarder(name) {}
}
78 changes: 78 additions & 0 deletions contracts/mocks/CallReceiverMock.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.24;

import "@openzeppelin/contracts/access/Ownable.sol";

contract CallReceiverMock is Ownable {
event MockFunctionCalled();
event MockFunctionCalledWithArgs(uint256 a, uint256 b);

uint256[] private _array;

constructor() payable Ownable(msg.sender){}

function mockFunction() public payable returns (string memory) {
emit MockFunctionCalled();
return "0x1234";
}

function mockFunctionEmptyReturn() public payable {
emit MockFunctionCalled();
}

function mockFunctionWithArgs(uint256 a, uint256 b) public payable returns (string memory) {
emit MockFunctionCalledWithArgs(a, b);
return "0x1234";
}

function mockFunctionNonPayable() public returns (string memory) {
emit MockFunctionCalled();
return "0x1234";
}

function mockStaticFunction() public pure returns (string memory) {
return "0x1234";
}

function mockFunctionRevertsNoReason() public payable {
revert();
}

function mockFunctionRevertsReason() public payable {
revert("CallReceiverMock: reverting");
}

function mockFunctionThrows() public payable {
assert(false);
}

function mockFunctionOutOfGas() public payable {
for (uint256 i = 0; ; ++i) {
_array.push(i);
}
}

function mockFunctionWritesStorage(bytes32 slot, bytes32 value) public returns (string memory) {
assembly {
sstore(slot, value)
}
return "0x1234";
}

function withdraw() external onlyOwner {
payable(owner()).transfer(address(this).balance);
}
}

contract CallReceiverMockTrustingForwarder is CallReceiverMock {
address private _trustedForwarder;

constructor(address trustedForwarder_) {
_trustedForwarder = trustedForwarder_;
}

function isTrustedForwarder(address forwarder) public view virtual returns (bool) {
return forwarder == _trustedForwarder;
}
}
2 changes: 1 addition & 1 deletion ignition/modules/main.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { buildModule } from '@nomicfoundation/hardhat-ignition/modules';

const GenericTokenMetaModule = buildModule('GenericTokenMetaModule', (m) => {
const forwarder = m.contract('Forwarder');
const forwarder = m.contract('Forwarder', ['Forwarder']);
const token = m.contract('GenericTokenMeta', [
'GenericERC20',
'GT',
Expand Down
2 changes: 0 additions & 2 deletions remappings.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1 @@
@openzeppelin/=node_modules/@openzeppelin/
forge-std/=lib/forge-std/src/
hardhat/=node_modules/hardhat/
57 changes: 0 additions & 57 deletions subgraph/datasources/forwarder.gql.json

This file was deleted.

48 changes: 0 additions & 48 deletions subgraph/datasources/forwarder.ts

This file was deleted.

22 changes: 0 additions & 22 deletions subgraph/datasources/forwarder.yaml

This file was deleted.

8 changes: 0 additions & 8 deletions subgraph/fetch/account.ts

This file was deleted.

19 changes: 0 additions & 19 deletions subgraph/fetch/forwarder.ts

This file was deleted.

6 changes: 0 additions & 6 deletions subgraph/subgraph.config.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,6 @@
"address": "0x0000000000000000000000000000000000000000",
"startBlock": 0,
"module": ["erc20", "pausable", "accesscontrol"]
},
{
"name": "Forwarder",
"address": "0x0000000000000000000000000000000000000000",
"startBlock": 0,
"module": ["forwarder"]
}
]
}
Loading

0 comments on commit 232633e

Please sign in to comment.