Tadle
Tadle
DeFiFoundry
27,750 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Addresses hash collisions leads to DOS

dinkras

Summary

The protocol uses virtual addresses as keys to store allmost all of the data. Those addresses are generated via the GenerateAddress lib. However collision of addresses is possible due to the nature of the custom address generation algorithm. This might lead to DOS of some of PreMarket functions.

Vulnerability Details

PreMarkets generates virtual addresses based on the offerId value.

function createOffer(CreateOfferParams calldata params) external payable {
// skipped code above
address makerAddr = GenerateAddress.generateMakerAddress(offerId);
address offerAddr = GenerateAddress.generateOfferAddress(offerId);
address stockAddr = GenerateAddress.generateStockAddress(offerId);

The specific algorithm for generating those addreses is the following

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/libraries/GenerateAddress.sol#L17

/// @dev Generate address for offer address with id
function generateOfferAddress(uint256 _id) internal pure returns (address) {
return address(uint160(uint256(keccak256(abi.encode(_id, "offer")))));
}

Consider this scenario:

A and B are 2 different _id args passed to generateOfferAddress()

The result A of uint256(keccak256(abi.encode(_id, "offer")))) is : 777

The result B of uint256(keccak256(abi.encode(_id, "offer")))) is : uint160.max + 778

When the result B(uint160.max + 778) is casted to uint160(next step of the algo) it overflows and the value becomes: 777

Now the 2 different inputs, produce the same address(numerical value)-> Hash collision

Impact

DOS of PreMarkets.createOffer() , listOffer(), createTaker(). This is due to the validation in those functions. They revert if the address exists

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/PreMarkets.sol#L67-L81

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/PreMarkets.sol#L206-L209

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/PreMarkets.sol#L365-L367

if (makerInfoMap[makerAddr].authority != address(0x0)) {
revert MakerAlreadyExist();
}
​
if (offerInfoMap[offerAddr].authority != address(0x0)) {
revert OfferAlreadyExist();
}
​
if (stockInfoMap[stockAddr].authority != address(0x0)) {
revert StockAlreadyExist();
}

Tools Used

Manual review

Recommendations

Consider using uint256 as ids for the data structures, instead of addresses.

Updates

Lead Judging Commences

0xnevi Lead Judge
over 1 year ago
0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!