Direct Transfer

The payableMagnaDirectAirdrop.disperseNativeToken() function should allow users to airdrop native tokens by sending them with the call.

The function records the balance of the contract before assets are send, including the msg.value which should be distributed`

uint256 tokenBalanceBefore = address(this).balance;

Then it transfers the tokens to the recipients and finally performs the following check

require(address(this).balance - tokenBalanceBefore == 0);

The idea of the check is to ensure exactly the desired amount was sent to the recipients, but because tokenBalanceBefore includes msg.value, the calculation will always underflow. In result, distributing native tokens with the contract is impossible.

Exclude the msg.value from the contract balance when taking the first snapshot.

- uint256 tokenBalanceBefore = address(this).balance;
+ uint256 tokenBalanceBefore = address(this).balance - msg.value;

MagnaDirectAirdropFactory.createDirectAirdropContract() is not permissioned, which allows any user to create contracts for themselves, with owner being an address chosen by them. A malicious user can DOS a factory by deploying as much contracts as needed until directAirdropContracts.length == maximumNumberOfContracts. Then nobody will be able to deploy more contracts from that factory.

Furthermore, if maximumNumberOfContracts is set to a high value, letting anyone create contracts raises the risk of getDirectAirdropContracts() reverting because of OOG.

Consider having an owner set during deployment of the factory and allow only them to deploy contracts.

The constructor of the factory contract accepts maximumNumberOfContracts_ parameter that's used to validate the number of created instances via this factory. Since there is no validation performed on this parameter, factories with 0 allowed contracts can be deployed. Such factories don't serve any purpose because they cannot deploy even a single contract.

On the other hand, it's also possible to create a factory with maximumNumberOfContracts_ = type(uint256.max). This can cause an OOG revert for the getDirectAirdropContracts() function if the instances are too much and lead to reverts for third-party integrators.

Consider bounding the _maximumNumberOfContracts between 1 and an appropriate maximum value.

After each airdrop dispersal, it's enforced as an invariant that all of the funds received from the sender are spent. This opens up the possibility of DOS-ing the airdrop via malicious receiver for tokens with hooks.

In disperseNativeToken() the following is mentioned: // 1. recipients intentionally reverting should be removed from the airdop

However, this case is slightly different because the recipient itself is not reverting, they are just sending 1 wei of the token back to the contract, which will make it seem like they were a legit recipient, while the transaction will later revert because of the following check:

require(token.balanceOf(address(this)) - tokenBalanceBefore == 0);

Depending on how such failures are handled - for example, if there is an automated process which checks for recipients that caused the revert and submits the dispersal again - this behavior may cause unexpected results. The described scenario cannot happen with native tokens because the contract reverts

Keep that in mind when you explore reverting airdrops.

Whenever a dispersal is performed, the AirdropCompleted(idempotencyKey, msgSender) event is emitted. Only idempotencyKey and msgSender and no token are being emitted, which may not be enough for offchain listeners to parse the dispersal. In addition, airdrops can be performed by anyone and any tokens can be supplied, so it's preferable to include the token address as well.

Include the token address in the event emission.

The Address library from OpenZeppelin is imported in the MagnaDirectAirdrop contract, but its functionality is never used.

Remove the library from the contract.

Any tokens with hooks staying in the contract can be flashloaned in the following way:

• call disperseERC20Token() with yourself as the recipient, total = 0 and value = token.balanceOf(contract).
• Since total is 0, you don't pay anything, but receive the whole balance
• Perform some actions and return the funds

Document this behavior.

Each disperse function computes recipientsLength = recipients.length - 1 before validating recipients.length == values.length. When recipients.length == 0, this underflows and reverts with a Panic error, yielding a non-descriptive failure and bypassing the intended input validation path.

Add an explicit require(recipients.length > 0, 'Empty') (or a custom error) before subtracting 1, and then keep the existing length-equality check. This avoids unexpected Panic errors and improves diagnosability.

The following comments are not accurate:

1. MagnaDirectAirdrop.sol#L101 - it's in the safeDisperseERC20Token() and says safeTransfer is not used which is not true. The comment was probably copied from disperseERC20Token() and forgotten.

1. Delete the comment