search

code-423n4 / 2022-06-nibbl-findings

1 stars 0 forks source link

Address of `NibblVault` created by `NibblVaultFactory` can be duplicated #305

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-06-nibbl/blob/8c3dbd6adf350f35c58b31723d42117765644110/contracts/NibblVaultFactory.sol#L50

Vulnerability details

Impact

In NibbleVaultFactory.createVault() function, the proxy vault is deployed using CREATE2 opcode. This allows the ability to predict the address where the contract will be deployed. The address is calculated as

new_address = hash(0xFF, sender, salt, bytecode)

And in this implementation, the salt is computed only using parameters of the vault which can be duplicated and make the second deployment fail.

Proof of concept

Scenario

  1. Alice created a vault with his NFT with the set of params X
  2. After her vault was buyouted by Bob, Bob sold it to the market and she occasionally bought it.
  3. Because the NFT has not changed value, she creates a vault with same set of params X. But this time she is unable to create the vault because the vault with set of params X is already existed.

Tools Used

Manual Review

Recommended Mitigation Steps

Add a nonce for each user or allow users to pass in a _mix like in createBasket() function.

HardlyDifficult commented 2 years ago

Adding nonce for this is a reasonable suggestion, without this the user could make a minor change to the input params so they were not really blocked. So lowering the risk here and merging with the warden's QA report #310