MohammedRizwan - In OrderBook.sol contract, Use of deprecated draft-EIP712Upgradeable.sol cause security issues

[sherlock-admin]

MohammedRizwan

high

In OrderBook.sol contract, Use of deprecated draft-EIP712Upgradeable.sol cause security issues

Summary

In OrderBook.sol contract, Use of deprecated draft-EIP712Upgradeable.sol cause security issues

Vulnerability Detail

Impact

In OrderBook.sol,

import { EIP712Upgradeable } from "@openzeppelin/contracts-upgradeable/utils/cryptography/draft-EIP712Upgradeable.sol";

The contract has used deprecated draft-EIP712Upgradeable.sol contract which is from openzeppelin. The contracts has used openzeppelin v4.5.0 which is too old.

Openzeppelin has deprecated draft-EIP712Upgradeable.sol contract in v4.8.0. It says,

Deprecations
EIP712: Added the file EIP712.sol and deprecated draft-EIP712.sol since the EIP is no longer a Draft. Developers are encouraged to update their imports. (OpenZeppelin/openzeppelin-contracts#3621)

-import "@openzeppelin/contracts/utils/cryptography/draft-EIP712.sol";
+import "@openzeppelin/contracts/utils/cryptography/EIP712.sol";

Reference link- https://github.com/OpenZeppelin/openzeppelin-contracts/releases

After openzeppelin version- 4.5.0, There were lots of security patches and code optimization happened in EIP712Upgradeable.sol.
Some references as below,
OpenZeppelin/openzeppelin-contracts#3969

V4.9.0 Breaking changes include,

Breaking changes
EIP712: Addition of ERC5267 support requires support for user defined value types, which was released in Solidity version 0.8.8. This requires a pragma change from ^0.8.0 to ^0.8.8.
EIP712: Optimization of the cache for the upgradeable version affects the way name and version are set. This is no longer done through an initializer, and is instead part of the implementation's constructor. As a consequence, all proxies using the same implementation will necessarily share the same name and version. Additionally, an implementation upgrade risks changing the EIP712 domain unless the same name and version are used when deploying the new implementation contract.

Code Snippet

https://github.com/hubble-exchange/hubble-protocol/blob/d89714101dd3494b132a3e3f9fed9aca4e19aef6/contracts/orderbooks/OrderBook.sol#L6

Tool used

Manual Review

Recommendation

Use openzeppelin EIP712Upgradeable.sol contract with latest version. Contract link