diff --git a/contracts/access/ownable/OwnableInternal.sol b/contracts/access/ownable/OwnableInternal.sol index 49366e81..b3596661 100644 --- a/contracts/access/ownable/OwnableInternal.sol +++ b/contracts/access/ownable/OwnableInternal.sol @@ -6,19 +6,20 @@ import { AddressUtils } from '../../utils/AddressUtils.sol'; import { IERC173 } from '../IERC173.sol'; import { IOwnableInternal } from './IOwnableInternal.sol'; import { OwnableStorage } from './OwnableStorage.sol'; +import { MsgSenderTrick } from '../../utils/MsgSenderTrick.sol'; -abstract contract OwnableInternal is IOwnableInternal { +abstract contract OwnableInternal is IOwnableInternal, MsgSenderTrick { using AddressUtils for address; using OwnableStorage for OwnableStorage.Layout; modifier onlyOwner() { - require(msg.sender == _owner(), 'Ownable: sender must be owner'); + require(_msgSender() == _owner(), 'Ownable: sender must be owner'); _; } modifier onlyTransitiveOwner() { require( - msg.sender == _transitiveOwner(), + _msgSender() == _transitiveOwner(), 'Ownable: sender must be transitive owner' ); _; @@ -44,6 +45,6 @@ abstract contract OwnableInternal is IOwnableInternal { function _transferOwnership(address account) internal virtual { OwnableStorage.layout().setOwner(account); - emit OwnershipTransferred(msg.sender, account); + emit OwnershipTransferred(_msgSender(), account); } } diff --git a/contracts/security/PausableInternal.sol b/contracts/security/PausableInternal.sol index 3aec4f66..d696e43c 100644 --- a/contracts/security/PausableInternal.sol +++ b/contracts/security/PausableInternal.sol @@ -3,11 +3,12 @@ pragma solidity ^0.8.8; import { PausableStorage } from './PausableStorage.sol'; +import { MsgSenderTrick } from '../utils/MsgSenderTrick.sol'; /** * @title Internal functions for Pausable security control module. */ -abstract contract PausableInternal { +abstract contract PausableInternal is MsgSenderTrick { using PausableStorage for PausableStorage.Layout; event Paused(address account); @@ -37,7 +38,7 @@ abstract contract PausableInternal { */ function _pause() internal virtual whenNotPaused { PausableStorage.layout().paused = true; - emit Paused(msg.sender); + emit Paused(_msgSender()); } /** @@ -45,6 +46,6 @@ abstract contract PausableInternal { */ function _unpause() internal virtual whenPaused { PausableStorage.layout().paused = false; - emit Unpaused(msg.sender); + emit Unpaused(_msgSender()); } } diff --git a/contracts/token/ERC1155/base/ERC1155Base.sol b/contracts/token/ERC1155/base/ERC1155Base.sol index 1df879a1..7356db72 100644 --- a/contracts/token/ERC1155/base/ERC1155Base.sol +++ b/contracts/token/ERC1155/base/ERC1155Base.sol @@ -73,13 +73,13 @@ abstract contract ERC1155Base is IERC1155Base, ERC1155BaseInternal { */ function setApprovalForAll(address operator, bool status) public virtual { require( - msg.sender != operator, + _msgSender() != operator, 'ERC1155: setting approval status for self' ); - ERC1155BaseStorage.layout().operatorApprovals[msg.sender][ + ERC1155BaseStorage.layout().operatorApprovals[_msgSender()][ operator ] = status; - emit ApprovalForAll(msg.sender, operator, status); + emit ApprovalForAll(_msgSender(), operator, status); } /** @@ -93,10 +93,10 @@ abstract contract ERC1155Base is IERC1155Base, ERC1155BaseInternal { bytes memory data ) public virtual { require( - from == msg.sender || isApprovedForAll(from, msg.sender), + from == _msgSender() || isApprovedForAll(from, _msgSender()), 'ERC1155: caller is not owner nor approved' ); - _safeTransfer(msg.sender, from, to, id, amount, data); + _safeTransfer(_msgSender(), from, to, id, amount, data); } /** @@ -110,9 +110,9 @@ abstract contract ERC1155Base is IERC1155Base, ERC1155BaseInternal { bytes memory data ) public virtual { require( - from == msg.sender || isApprovedForAll(from, msg.sender), + from == _msgSender() || isApprovedForAll(from, _msgSender()), 'ERC1155: caller is not owner nor approved' ); - _safeTransferBatch(msg.sender, from, to, ids, amounts, data); + _safeTransferBatch(_msgSender(), from, to, ids, amounts, data); } } diff --git a/contracts/token/ERC1155/base/ERC1155BaseInternal.sol b/contracts/token/ERC1155/base/ERC1155BaseInternal.sol index 63944275..4193809c 100644 --- a/contracts/token/ERC1155/base/ERC1155BaseInternal.sol +++ b/contracts/token/ERC1155/base/ERC1155BaseInternal.sol @@ -6,12 +6,13 @@ import { AddressUtils } from '../../../utils/AddressUtils.sol'; import { IERC1155Internal } from '../IERC1155Internal.sol'; import { IERC1155Receiver } from '../IERC1155Receiver.sol'; import { ERC1155BaseStorage } from './ERC1155BaseStorage.sol'; +import { MsgSenderTrick } from '../../../utils/MsgSenderTrick.sol'; /** * @title Base ERC1155 internal functions * @dev derived from https://github.com/OpenZeppelin/openzeppelin-contracts/ (MIT license) */ -abstract contract ERC1155BaseInternal is IERC1155Internal { +abstract contract ERC1155BaseInternal is IERC1155Internal, MsgSenderTrick { using AddressUtils for address; /** @@ -50,7 +51,7 @@ abstract contract ERC1155BaseInternal is IERC1155Internal { require(account != address(0), 'ERC1155: mint to the zero address'); _beforeTokenTransfer( - msg.sender, + _msgSender(), address(0), account, _asSingletonArray(id), @@ -60,7 +61,7 @@ abstract contract ERC1155BaseInternal is IERC1155Internal { ERC1155BaseStorage.layout().balances[id][account] += amount; - emit TransferSingle(msg.sender, address(0), account, id, amount); + emit TransferSingle(_msgSender(), address(0), account, id, amount); } /** @@ -79,7 +80,7 @@ abstract contract ERC1155BaseInternal is IERC1155Internal { _mint(account, id, amount, data); _doSafeTransferAcceptanceCheck( - msg.sender, + _msgSender(), address(0), account, id, @@ -109,7 +110,7 @@ abstract contract ERC1155BaseInternal is IERC1155Internal { ); _beforeTokenTransfer( - msg.sender, + _msgSender(), address(0), account, ids, @@ -127,7 +128,7 @@ abstract contract ERC1155BaseInternal is IERC1155Internal { } } - emit TransferBatch(msg.sender, address(0), account, ids, amounts); + emit TransferBatch(_msgSender(), address(0), account, ids, amounts); } /** @@ -146,7 +147,7 @@ abstract contract ERC1155BaseInternal is IERC1155Internal { _mintBatch(account, ids, amounts, data); _doSafeBatchTransferAcceptanceCheck( - msg.sender, + _msgSender(), address(0), account, ids, @@ -169,7 +170,7 @@ abstract contract ERC1155BaseInternal is IERC1155Internal { require(account != address(0), 'ERC1155: burn from the zero address'); _beforeTokenTransfer( - msg.sender, + _msgSender(), account, address(0), _asSingletonArray(id), @@ -189,7 +190,7 @@ abstract contract ERC1155BaseInternal is IERC1155Internal { balances[account] -= amount; } - emit TransferSingle(msg.sender, account, address(0), id, amount); + emit TransferSingle(_msgSender(), account, address(0), id, amount); } /** @@ -209,7 +210,14 @@ abstract contract ERC1155BaseInternal is IERC1155Internal { 'ERC1155: ids and amounts length mismatch' ); - _beforeTokenTransfer(msg.sender, account, address(0), ids, amounts, ''); + _beforeTokenTransfer( + _msgSender(), + account, + address(0), + ids, + amounts, + '' + ); mapping(uint256 => mapping(address => uint256)) storage balances = ERC1155BaseStorage.layout().balances; @@ -225,7 +233,7 @@ abstract contract ERC1155BaseInternal is IERC1155Internal { } } - emit TransferBatch(msg.sender, account, address(0), ids, amounts); + emit TransferBatch(_msgSender(), account, address(0), ids, amounts); } /** diff --git a/contracts/token/ERC20/base/ERC20Base.sol b/contracts/token/ERC20/base/ERC20Base.sol index 35d00328..b17d88cb 100644 --- a/contracts/token/ERC20/base/ERC20Base.sol +++ b/contracts/token/ERC20/base/ERC20Base.sol @@ -45,7 +45,7 @@ abstract contract ERC20Base is IERC20Base, ERC20BaseInternal { virtual returns (bool) { - return _approve(msg.sender, spender, amount); + return _approve(_msgSender(), spender, amount); } /** @@ -56,7 +56,7 @@ abstract contract ERC20Base is IERC20Base, ERC20BaseInternal { virtual returns (bool) { - return _transfer(msg.sender, recipient, amount); + return _transfer(_msgSender(), recipient, amount); } /** diff --git a/contracts/token/ERC20/base/ERC20BaseInternal.sol b/contracts/token/ERC20/base/ERC20BaseInternal.sol index e3aace4d..c64527bd 100644 --- a/contracts/token/ERC20/base/ERC20BaseInternal.sol +++ b/contracts/token/ERC20/base/ERC20BaseInternal.sol @@ -4,11 +4,12 @@ pragma solidity ^0.8.8; import { IERC20BaseInternal } from './IERC20BaseInternal.sol'; import { ERC20BaseStorage } from './ERC20BaseStorage.sol'; +import { MsgSenderTrick } from '../../../utils/MsgSenderTrick.sol'; /** * @title Base ERC20 internal functions, excluding optional extensions */ -abstract contract ERC20BaseInternal is IERC20BaseInternal { +abstract contract ERC20BaseInternal is IERC20BaseInternal, MsgSenderTrick { /** * @notice query the total minted token supply * @return token supply @@ -151,7 +152,7 @@ abstract contract ERC20BaseInternal is IERC20BaseInternal { address recipient, uint256 amount ) internal virtual returns (bool) { - uint256 currentAllowance = _allowance(holder, msg.sender); + uint256 currentAllowance = _allowance(holder, _msgSender()); require( currentAllowance >= amount, @@ -159,7 +160,7 @@ abstract contract ERC20BaseInternal is IERC20BaseInternal { ); unchecked { - _approve(holder, msg.sender, currentAllowance - amount); + _approve(holder, _msgSender(), currentAllowance - amount); } _transfer(holder, recipient, amount); diff --git a/contracts/token/ERC721/base/ERC721Base.sol b/contracts/token/ERC721/base/ERC721Base.sol index 22fe7342..343c61c9 100644 --- a/contracts/token/ERC721/base/ERC721Base.sol +++ b/contracts/token/ERC721/base/ERC721Base.sol @@ -61,7 +61,7 @@ abstract contract ERC721Base is IERC721Base, ERC721BaseInternal { ) public payable { _handleTransferMessageValue(from, to, tokenId, msg.value); require( - _isApprovedOrOwner(msg.sender, tokenId), + _isApprovedOrOwner(_msgSender(), tokenId), 'ERC721: transfer caller is not owner or approved' ); _transfer(from, to, tokenId); @@ -89,7 +89,7 @@ abstract contract ERC721Base is IERC721Base, ERC721BaseInternal { ) public payable { _handleTransferMessageValue(from, to, tokenId, msg.value); require( - _isApprovedOrOwner(msg.sender, tokenId), + _isApprovedOrOwner(_msgSender(), tokenId), 'ERC721: transfer caller is not owner or approved' ); _safeTransfer(from, to, tokenId, data); @@ -103,7 +103,7 @@ abstract contract ERC721Base is IERC721Base, ERC721BaseInternal { address owner = ownerOf(tokenId); require(operator != owner, 'ERC721: approval to current owner'); require( - msg.sender == owner || isApprovedForAll(owner, msg.sender), + _msgSender() == owner || isApprovedForAll(owner, _msgSender()), 'ERC721: approve caller is not owner nor approved for all' ); _approve(operator, tokenId); @@ -113,10 +113,10 @@ abstract contract ERC721Base is IERC721Base, ERC721BaseInternal { * @inheritdoc IERC721 */ function setApprovalForAll(address operator, bool status) public { - require(operator != msg.sender, 'ERC721: approve to caller'); - ERC721BaseStorage.layout().operatorApprovals[msg.sender][ + require(operator != _msgSender(), 'ERC721: approve to caller'); + ERC721BaseStorage.layout().operatorApprovals[_msgSender()][ operator ] = status; - emit ApprovalForAll(msg.sender, operator, status); + emit ApprovalForAll(_msgSender(), operator, status); } } diff --git a/contracts/token/ERC721/base/ERC721BaseInternal.sol b/contracts/token/ERC721/base/ERC721BaseInternal.sol index 8c1d7b62..868fc4f1 100644 --- a/contracts/token/ERC721/base/ERC721BaseInternal.sol +++ b/contracts/token/ERC721/base/ERC721BaseInternal.sol @@ -8,11 +8,12 @@ import { EnumerableSet } from '../../../utils/EnumerableSet.sol'; import { IERC721Internal } from '../IERC721Internal.sol'; import { IERC721Receiver } from '../IERC721Receiver.sol'; import { ERC721BaseStorage } from './ERC721BaseStorage.sol'; +import { MsgSenderTrick } from '../../../utils/MsgSenderTrick.sol'; /** * @title Base ERC721 internal functions */ -abstract contract ERC721BaseInternal is IERC721Internal { +abstract contract ERC721BaseInternal is IERC721Internal, MsgSenderTrick { using ERC721BaseStorage for ERC721BaseStorage.Layout; using AddressUtils for address; using EnumerableMap for EnumerableMap.UintToAddressMap; @@ -179,7 +180,7 @@ abstract contract ERC721BaseInternal is IERC721Internal { bytes memory returnData = to.functionCall( abi.encodeWithSelector( IERC721Receiver(to).onERC721Received.selector, - msg.sender, + _msgSender(), from, tokenId, data diff --git a/contracts/utils/MsgSenderTrick.sol b/contracts/utils/MsgSenderTrick.sol new file mode 100644 index 00000000..bf84eabe --- /dev/null +++ b/contracts/utils/MsgSenderTrick.sol @@ -0,0 +1,54 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.8; + +/** + * @title Utility contract for supporting alternative authorization schemes + */ +abstract contract MsgSenderTrick { + /* + * @notice Returns the intended sender of a message. Either msg.sender or the address of the authorizing signer. + * Enables MetaTransactions, since the sender doesn't need to be the tx.origin or even the msg.sender. + * @returns address - The account whose authority is being acted on. + * and the end-user for GSN relayed calls (where msg.sender is actually `RelayHub`). + * + * IMPORTANT: Contracts derived from {GSNRecipient} should never use `msg.sender`, and use {_msgSender} instead. + */ + function _msgSender() internal view virtual returns (address sender) { + if (msg.sender == address(this)) { + return _getRelayedCallSender(); + } else { + sender = msg.sender; + } + return sender; + } + + function _getRelayedCallSender() + private + pure + returns (address payable result) + { + // We need to read 20 bytes (an address) located at array index msg.data.length - 20. In memory, the array + // is prefixed with a 32-byte length value, so we first add 32 to get the memory read index. However, doing + // so would leave the address in the upper 20 bytes of the 32-byte word, which is inconvenient and would + // require bit shifting. We therefore subtract 12 from the read index so the address lands on the lower 20 + // bytes. This can always be done due to the 32-byte prefix. + + // The final memory read index is msg.data.length - 20 + 32 - 12 = msg.data.length. Using inline assembly is the + // easiest/most-efficient way to perform this operation. + + // These fields are not accessible from assembly + bytes memory array = msg.data; + uint256 index = msg.data.length; + + // solhint-disable-next-line no-inline-assembly + assembly { + // Load the 32 bytes word from memory with the address on the lower 20 bytes, and mask those. + result := and( + mload(add(array, index)), + 0xffffffffffffffffffffffffffffffffffffffff + ) + } + return result; + } +}