Omniscia LimeChain Audit

ERC20Permit Manual Review Findings

ERC20Permit Manual Review Findings

ERC-01M: Insecure Usage of Assembly

Description:

The permit function of the ERC20Permit contract implements the permit workflow in in-line assembly, calculating keccak256 hashes twice. This can lead to improper hash values due to a compiler bug that was fixed in version 0.8.3 but affects the current project as it uses 0.8.0.

Example:

contracts/ERC20Permit.sol
64assembly {
65 // Load free memory pointer
66 let memPtr := mload(64)
67
68 // keccak256("Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)")
69 mstore(
70 memPtr,
71 0x6e71edae12b1b97f4d1f60370fef10105fa2faae0126114a169c64845d6126c9
72 )
73 mstore(add(memPtr, 32), _owner)
74 mstore(add(memPtr, 64), _spender)
75 mstore(add(memPtr, 96), _amount)
76 mstore(add(memPtr, 128), nonce)
77 mstore(add(memPtr, 160), _deadline)
78
79 hashStruct := keccak256(memPtr, 192)
80}
81
82bytes32 eip712DomainHash = _domainSeparator();
83
84// Assembly for more efficient computing:
85// bytes32 hash = keccak256(
86// abi.encodePacked(uint16(0x1901), eip712DomainHash, hashStruct)
87// );
88
89bytes32 hash;
90
91assembly {
92 // Load free memory pointer
93 let memPtr := mload(64)
94
95 mstore(
96 memPtr,
97 0x1901000000000000000000000000000000000000000000000000000000000000
98 ) // EIP191 header
99 mstore(add(memPtr, 2), eip712DomainHash) // EIP712 domain hash
100 mstore(add(memPtr, 34), hashStruct) // Hash of struct
101
102 hash := keccak256(memPtr, 66)
103}

Recommendation:

We advise the assembly blocks to be ultimately removed from the codebase as the gas optimization should be negligible versus the traditional permit workflow as implemented by the draft ERC20Permit of OpenZeppelin. In any case, if assembly is desired we advise the compiler version to be increased.

Alleviation:

The contract was omitted from the codebase and replaced by the OpenZeppelin draft implementation thus rendering this exhibit null.