Project
One World
NFTDeFi
15,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

external caller can impersonate any user, can grief any user into joinDAO() calls even if it's against the user's wishes and permission

copperscrewer

Summary

The joinDAO method requires an user to have approved funds from the currency token to the protocol's contract address. The external caller role can easily steal allowances by creating a DAO with suitable tier price and steal funds.

callExternalContract to the contract address itself

https://github.com/Cyfrin/2024-11-one-world/blob/1e872c7ab393c380010a507398d4b4caca1ae32b/contracts/dao/MembershipFactory.sol#L176C14-L179

Impersonate any user

https://github.com/Cyfrin/2024-11-one-world/blob/1e872c7ab393c380010a507398d4b4caca1ae32b/contracts/dao/MembershipFactory.sol#L182-L202

Vulnerability Details

In _msgSender() we have msg.data being able to impersonate any person/user. This is reachable when the EXTERNAL_CALLER uses callExternalContract() to the same contract address

function _msgSender()
internal
view
override
returns (address sender)
{
if (msg.sender == address(this)) {
bytes memory array = msg.data;
uint256 index = msg.data.length;
assembly {
// Load the 32 bytes word from memory with the address on the lower 20 bytes, and mask those.
sender := and(
mload(add(array, index)),
0xffffffffffffffffffffffffffffffffffffffff
)
}
} else {

In joinDAO, we have code to transfer from allowances for _msgSender() which is proved that EXTERNAL_CALLER can spoof to any value.

function joinDAO(address daoMembershipAddress, uint256 tierIndex) external {
.
.
.
IERC20(daos[daoMembershipAddress].currency).transferFrom(_msgSender(), owpWallet, platformFees);
IERC20(daos[daoMembershipAddress].currency).transferFrom(_msgSender(), daoMembershipAddress, tierPrice - platformFees);
IMembershipERC1155(daoMembershipAddress).mint(_msgSender(), tierIndex, 1);

Impact

User funds can be stolen and are at constant risk from the address of EXTERNAL_CALLER

Tools Used

manual analysis

Recommendations

joining a DAO should be entirely upto the user so use msg.sender instead of _msgSender

Updates

Lead Judging Commences

0xbrivan2 Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
0xbrivan2 Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Appeal created

copperscrewer Submitter
10 months ago
0xbrivan2 Lead Judge
10 months ago
0xbrivan2 Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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