Cyfrin / 2023-09-sparkn-mitigation2

Other
0 stars 0 forks source link

M-01 NotFixed #12

Open jksgfsdfd opened 1 year ago

jksgfsdfd commented 1 year ago

Issue Details

M-01 : The digest calculation in deployProxyAndDistributeBySignature does not follow EIP-712 specification

Issue Link : https://www.codehawks.com/report/cllcnja1h0001lc08z7w0orxx#M-01

Fix PR : https://github.com/codefox-inc/sparkn-contracts/pull/29

Review

Not Fixed.

EIP-712 requires dynamic types to be encoded as the hash of its content when calculating the digest. The updated code still encodes bytes data as bytes.

Suggestion

Update the code in the deployProxyAndDistributeBySignature() function https://github.com/codefox-inc/sparkn-contracts/blob/ec4d336d0df7c927e4769860717716a9f3b4199f/src/ProxyFactory.sol#L168C1-L170C104

diff --git a/src/ProxyFactory.sol b/src/ProxyFactory.sol
index d7a1960..c77559a 100644
--- a/src/ProxyFactory.sol
+++ b/src/ProxyFactory.sol
@@ -165,8 +165,9 @@ contract ProxyFactory is Ownable, EIP712 {
         bytes calldata signature,
         bytes calldata data
     ) public returns (address) {
-        bytes32 digest =
-            _hashTypedDataV4(keccak256(abi.encode(_DEPLOY_AND_DISTRIBUTE_TYPEHASH, contestId, implementation, data)));
+        bytes32 digest = _hashTypedDataV4(
+            keccak256(abi.encode(_DEPLOY_AND_DISTRIBUTE_TYPEHASH, contestId, implementation, keccak256(data)))
+        );
         if (!organizer.isValidSignatureNow(digest, signature)) revert ProxyFactory__InvalidSignature();
         bytes32 salt = _calculateSalt(organizer, contestId, implementation);
         if (saltToCloseTime[salt] == 0) revert ProxyFactory__ContestIsNotRegistered();