Sparkn

CodeFox Inc.
DeFiFoundryProxy
15,000 USDC
View results
Previous Next
Submission Details
Severity: medium

Same salt generation due to abi.encode collision

0xumarkhatab

Summary

abi.encode collision will result in the same salt being generated in setContest function.

Vulnerability Details

The contract uses salt generation for storing closeTime for contests or deploying them deterministically.
But abi.encode is prone to collisions as described in this Ethereum Exchange Post.

In setContest, even if the contest is not registered, just because it has the same salt , it will not get registered.

​
function setContest(address organizer, bytes32 contestId, uint256 closeTime, address implementation)
public
onlyOwner
{
..
..
bytes32 salt = _calculateSalt(organizer, contestId, implementation);
if (saltToCloseTime[salt] != 0) revert ProxyFactory__ContestIsAlreadyRegistered();
..
..
}
​

In deploy functions, this vulnerability will allow to deploy contracts who's salt is same as the one is already registered but might not be registered at the first place.

​
function deployProxyAndDistribute(bytes32 contestId, address implementation, bytes calldata data)
public
returns (address)
{
bytes32 salt = _calculateSalt(msg.sender, contestId, implementation);
@audit only checks if it exists , this might be the salt of existing contest
if (saltToCloseTime[salt] == 0) revert ProxyFactory__ContestIsNotRegistered();
​
}

PoC

Following both codes generate the same output using different static and dynamic data:

​
function collisionStatic() public view returns (bool) {
require(keccak256(abi.encode([1, 2], 3)) == keccak256(abi.encode(1, 2, 3)));
return true;
}
​
function collisionDynamic() public view returns (bool) {
bytes32 hash1 = keccak256(abi.encode(32, 1, 0x6100000000000000000000000000000000000000000000000000000000000000));
bytes32 hash2 = keccak256(abi.encode("a"));
require(hash1 == hash2);
return true;
}
​
​

Impact

  • overwriting closeTime of contests that were registered earlier but had the same salt as one of the new one.

Tools Used

Manual review, Ethereum exchange

Reference

https://ethereum.stackexchange.com/questions/113188/can-abi-encode-receive-different-values-and-return-the-same-result

Recommendations

Inspect the collision cases carefully and come up with solutions.
My recommendation would be to include user nonces and block.timestamp for reducing number of collisions

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.