Project

Impact

MembershipFactory::createNewDAOMembership can be front-run by a malicious user, who can set the config of dao, such that he can pass in arbitrary DAOInputConfig::ensname, and revert the tx of legitimate owner who owns the ensName.

Description

createNewDAOMembership takes in DAOInputConfig struct which has ensname as one of the arguments. But the problem with this function is it does not have any checks if the owner of the ensname has sent the tx, meaning that a malicious user can watch the mempool and frontrun the createNewDAOMembership function, so that when the legit owner of the ensname, transacts to create a new dao membership his, tx reverts because of existing one for that ensname.

Take a look at MembershipFactory.sol#L60.
Here you can see that the function has a require check that no existing mapping should be present for the given ensname.

require(getENSAddress[daoConfig.ensname] == address(0), "DAO already exist.");

And this one, MembershipFactory.sol#L90-L91

getENSAddress[daoConfig.ensname] = address(proxy); //@audit - updates the mapping for the ensname, in the front-run tx

//@audit - in the below line, malicious user becomes the creator of the dao under the ensname, he doesn't own

userCreatedDAOs[_msgSender()][daoConfig.ensname] = address(proxy);

Thus, when the tx execution for the legit user reaches this line MembershipFactory.sol#L60, his call will revert leading to DoS, since a dao has been created with his ensname, and owned by some other user.

NOTE - This can also happen by mistake from other legit users, even if no front-running is done by malicious user, because of arbitrary data passed to the function call and lack of checks.

Tools Used

Manual Review

Recommendation

Have checks in place to verify if the ensname is owned by the msg.sender, and if possible restrict the amount of arbitrary data passed to the function call.