Hedgey Finance

The function claimMultipleWithSig() is used by paymasters to claim tokens from multiple campaigns on behalf of a claimer. It uses a signature verification mechanism to control the access to this function and the ability to claim the claimer's tokens.The signature provided as an input parameter to this function includes several variables necessary to prevent signature attacks, e.g. signature replay, however, it does not do so effectively. When attempting to claim from multiple campaigns, the paymaster needs to provide a signature that, along with other variables, will verify the id and amount of tokens of the first campaign as well as the number of campaigns to claim. Solely relying on these variables opens up the possibility for the paymaster to claim unintended campaigns.As an example of a proof of concept:Claimer can claim from 4 campaigns;Claimer provides a proper signature to the paymaster to claim on his behalf for only 3 of the campaigns, specifically campaigns 1, 2, and 3.Paymaster uses the same signature to claim from campaigns 1, 2, and 4.The scenario identified previously is possible due to the fact that the first campaign matches what the claimer intended, and the number of claimed campaigns also matches. However, it does not match the intended campaigns that were meant to be claimed on behalf of the claimer.

The createPlan functions in contracts/VestingPlans/TokenVestingPlans.sol and contracts/VestingPlans/VotingTokenVestingPlans.sol do not validate vestingAdmin arguments, so callers can accidentally set important state variables to the zero address.This fact allows for plans to be created without the capability of being revoked.

The function transferFrom() is used by a vestingAdmin of a plan to transfer an NFT on behalf of its holder. The success of the execution of this function is determined by the toggle variable adminTransferOBO than can only be set by the holder of the NFT. As such, a vestingAdmin is hindered from acting rogue and transferring NFTs at will.However, a malicious or a compromised vestingAdmin that already has the transfer capability enabled may still transfer the plan to an arbitrary address, e.g. an address controlled by the malicious vestingAdmin.

The functions createUnlockedCampaign() and createLockedCampaign() do not perform the necessary validations on the variable donation.tokenLocker to ensure that a malicious address cannot be provided as input. The address pointed by this variable can be arbitrarily chosen by a malicious actor, and its allowance of the campaign token will be increased illegitimately.As an example of a proof of concept:Malicious actor creates contract to interact and exploit the ClaimCampaigns smart contract.Malicious actor creates a campaign providing donation tokens to be collected by the donationCollector. The variable donation.tokenLocker is set to the malicious contract created previously.The event TokensDonated() is emitted with a legitimate donationCollector.Since the malicious actor's allowance was increased by donation.amount, they can withdraw the donation back to themselves.Since the malicious actor also set themselves as the campaign.manager, they can cancel the campaign and reclaim the campaign.amount.The scenario identified above can be used to illegitimately force the emission of the TokensDonated() event that can be wrongfully ingested by other on-chain or off-chain components.

The function changeDonationcollector() and the constructor() do not validate the variables donationCollector and _tokenLockers against the Zero Address, allowing for undefined behavior within the protocol, and drainage of donation tokens.

The functions planBalanceOf() and planEnd() do not validate the variable planId against the Zero Value, allowing for incorrect information to be presented by the protocol. Even though a planId of 0 would most likely be invalid, the protocol can still confirm it exists even when it does not. Also it allows for divisions by 0 to occur.

It was observed that throughout the protocol there are multiple instances of redundant code on several accounts:Duplicated functions, e.g planBalanceOf() and planEnd();Similar functions with slight differences do not reuse code, e.g. createUnlockedCampaign() and createLockedCampaign();Similar contracts with slight differences do not use inheritance, e.g. TokenLockupPlans, VotingTokenLockupPlans, TokenVestingPlans and VotingTokenVestingPlans;These design patterns increase code complexity and do not maximize transaction gas and storage efficiency on the blockchain.

The function batchLockingPlans() performs the same Zero Address validation on the variable token already provided by other functions such as transferTokens(), safeIncreaseAllowance(), and createPlan().

The function _claimUnlockedTokens() performs the same validation twice on the same variable campaign.tokenLockup as the functions who call it. The first time always occurs within if conditions, and the second time occurs on _claimUnlockedTokens().

The function validateEnd() defines an output bool variable valid that identifies whether the execution of this function was successful. The usage of this variable is unnecessary since it is always set to true or the entire transaction reverts through the require statements.