Maga VP - audit 1

All of the MAGAVP tokens are sent to the contract deployer, which is an externally-owned account (EOA) address. This is a centralization risk because the deployer or the owner(s) of the EOAs can distribute tokens without obtaining the consensus of the community. Any compromise to these addresses may allow a hacker to steal and sell tokens on the market, resulting in severe damage to the project. ```sol=1057 uint256 totalTokenSupply = (50_000_000) * (10**18); _mint(owner(), totalTokenSupply); // only time internal mint function is ever called is to create supply ``` In addition, the comment says that the `_mint()` function is only called once in the constructor function. However, the `_setBalance()` function also calls the `_mint()` function, which allows the owner address to set the token balance of any user.

Fees are charged in the override `_transfer()` function of the `MAGAVP` contract. There is also a state variable `maxWallet` to make sure that each address does not exceed the maximum wallet token amount. ```sol=1059 maxWallet = totalTokenSupply / 50; // 2% ``` In the calculation of the `maxWallet`, our understanding is that the max holding of the receiver should be the sum of the holding tokens and the amount received from the transfer, i.e. `max holding >= already holding + newly received amount`. However, the current calculation of the max holding is calculating the fees as received amount, i.e. `max holding >= already holding + newly received amount + fees.` ```solidity=1355 require(contractBalanceRecipient + amount <= maxWallet,"Exceeds maximum wallet token amount." ); ``` - Max holding is checked. ```solidity=1376 amount = amount.sub(fees); ``` - Receiving amount is adjusted. ```solidity=326 _balances[recipient] = _balances[recipient].add(amount); ``` - Balance is increased by the adjusted amount.

In the function `swapAndSendDividends()` from the contract `MAGAVP`, there are three places where the `address(this).balance` is directly used. In the contract `MAGAVP`, the `receive()` function does not contain any logic. That is to say, the native address balance (`address(this).balance`) could be directly increased when receiving transactions with `msg.value`. Related calculations could be unexpectedly changed, since the increased balance is from neither buy nor sell. ```sol=1466 if (buyRewardsFee > 0) { dividendsFromBuy = address(this) .balance .mul(buyAmount) .div(totalAmount) .mul(buyRewardsFee) .div(buyRewardsFee + buyMarketingFees + buyBuyBackFee); } if (sellRewardsFee > 0) { dividendsFromSell = address(this) .balance .mul(sellAmount) .div(totalAmount) .mul(sellRewardsFee) .div(sellRewardsFee + sellMarketingFees + sellBuyBackFee); } ``` ```sol=1492 if (_completeFees > 0) { feePortions = address(this).balance.div(_completeFees); } ```

In the function call of `MAGAVP.processDividendTracker()` the function `dividendTracker.process()` will be called with a passed-in gas limit. However, in the execution of the `process()` function, if the input `gas` is not enough, the EVM may revert this call and return the out-of-gas exception. In addition, there is no access control on the `process()` function. Also, our understanding is that the users should claim their rewards themselves using the function `claim()`.

The `distributeDividends()` function does not account for the scenario where `totalSupply()` is equal to 0. In the current implementation, the function attempts to perform a division operation with `totalSupply` as the divisor. This will result in a division by zero error, which is a critical issue that can cause the function to revert and potentially lead to unexpected behavior in the contract. ```sol=787 function distributeDividends() public payable override { require(totalSupply() > 0); if (msg.value > 0) { magnifiedDividendPerShare = magnifiedDividendPerShare.add( (msg.value).mul(magnitude) / totalSupply() ); emit DividendsDistributed(msg.sender, msg.value); totalDividendsDistributed = totalDividendsDistributed.add( msg.value ); } } ```

In the constructor function of contract `MAGAVP`, there are a few lines of codes calling the `excludeFromDividends()` function to exclude the input addresses from receiving dividends. However, we noticed that on line 1045, `dividendTracker.excludedFromDividends(address(0));` the `excludeFromDividends()` function is not called correctly. Instead, the native getter function of the state variable `excludedFromDividends` is called. ```sol=1042 dividendTracker.excludeFromDividends(address(dividendTracker)); dividendTracker.excludeFromDividends(address(this)); dividendTracker.excludeFromDividends(DEAD); dividendTracker.excludedFromDividends(address(0)); dividendTracker.excludeFromDividends(router); dividendTracker.excludeFromDividends(marketingWallet); dividendTracker.excludeFromDividends(owner()); ```

In the `MAGAVP` contract, the `claim()` function does not properly handle the return values of the external function call. ```sol=1270 function claim() external { dividendTracker.processAccount(payable(msg.sender), false); } ``` ```sol=1844 function processAccount(address payable account, bool automatic) public onlyOwner returns (bool) ``` The `MAGAVP.claim()` function does not return anything and calls `dividendTracker.processAccount()` directly, while the `dividendTracker.processAccount()` function returns a `bool` value.

The function `swapTokensForEth()` doesn't check if the `tokenAmount` variable is above zero before executing. This could cause gas costs for each function call. ```sol=1394 function swapTokensForEth(uint256 tokenAmount) private { address[] memory path = new address[](2); path[0] = address(this); path[1] = uniswapV2Router.WETH(); _approve(address(this), address(uniswapV2Router), tokenAmount); uniswapV2Router.swapExactTokensForETHSupportingFeeOnTransferTokens( tokenAmount, 0, // accept any amount of ETH path, address(this), block.timestamp ); } ```

A sandwich attack may happen when an attacker observes a transaction swapping tokens or adding liquidity without setting restrictions on slippage or minimum output amount. The attacker can manipulate the exchange rate by frontrunning (executing before the target) a transaction to purchase one of the assets and make profits by backrunning (executing after the target) a transaction to sell the asset. The below functions call UniswapV2 interfaces without setting restrictions on slippage or minimum output amount, so transactions triggering the function are vulnerable to sandwich attacks, especially when the input amount is large. ```solidity=1399 uniswapV2Router.swapExactTokensForETHSupportingFeeOnTransferTokens( tokenAmount, 0, // accept any amount of ETH path, address(this), block.timestamp ); ``` Function `MAGAVP.swapTokensForEth` invokes `IUniswapV2Router02.swapExactTokensForETHSupportingFeeOnTransferTokens` without setting restrictions on slippage or minimum output amount, which may be triggered by anyone. --- ```sol=1912 uniswapV2Router .swapExactETHForTokensSupportingFeeOnTransferTokens{ value: _withdrawableDividend }( 0, // accept any amount of Tokens path, address(this), block.timestamp ) ``` Function `MAGAVP._reinvestDividendOfUser` invokes `IUniswapV2Router02.swapExactETHForTokensSupportingFeeOnTransferTokens` without setting restrictions on slippage or minimum output amount, which may be triggered by the owner address. --- ```sol=1972 uniswapV2Router .swapExactETHForTokensSupportingFeeOnTransferTokens{ value: _withdrawableDividend }( 0, // accept any amount of Tokens path, user, block.timestamp ) ``` Function `MAGAVP._withdrawDividendOfUser` invokes `IUniswapV2Router02.swapExactETHForTokensSupportingFeeOnTransferTokens` without setting restrictions on slippage or minimum output amount, which may be triggered by the owner address.

State variables that never change should be declared as `constant` to save gas. ```solidity=923 address public DEAD = 0x000000000000000000000000000000000000dEaD; ``` - `DEAD` should be declared `constant`.