Commit hash: 4d2599cfe1c7d2647c26750cf547631a896dd518

Audited by: Parth Patel (Parth#7949)

# Issues

## AccountToken

**[H-1]** No access control on `safeMint`. Prone to frontrunning attacks

On line [14-17], AccountToken.sol has the following code:

function safeMint(address _to, address _addr) public
{
_safeMint(_to, uint256(uint160(_addr)));
}

There is no access control on the creation of NFT. Any account can mint NFTs and become the owner of NFT corresponding to any address. This function is prone to frontrunning attacks. If some malicious user sees the transaction in mempool, they can front-run the transaction of minting the NFT and become the owner of that NFT causing the second transaction to fail.

Consider: Putting some access control on how users can become the owner of NFT.

**[H-2]** No access control on `mint`. Prone to frontrunning attacks

On line [19-22], AccountToken.sol has the following code:

function mint(address _to, address _addr) public
{
_mint(_to, uint256(uint160(_addr)));
}

There is no access control on the creation of NFT. Any account can mint NFTs and become the owner of NFT corresponding to any address. This function is prone to frontrunning attacks. If some malicious user sees the transaction in mempool, they can front-run the transaction of minting the NFT and become the owner of that NFT causing the second transaction to fail.

Consider: Putting some access control on how users can become the owner of NFT.

## AccountForwarder

**[H-1]** Minting of `AccountForwarder` can be prone to frontrunning resulting in anyone becoming the owner and contract creation failing.

On line [28-32], AccountToken.sol has the following code:

constructor(address _token)
{
token = AccountToken(_token);
token.safeMint(msg.sender, address(this));
}

Anyone can call `AccountToken.safeMint` or `AccountToken.mint` by passing the desired address and precalculating the address to the contract to be created. By doing this way, the NFT which should correspond to ownership of the `AccountForwarder`(proxy) account will be owned by a malicious user. And the deployment of the `AccountForwarder`(proxy) account will fail because NFT is already minted.

Consider: Allocating the NFTs id's incrementally so that it doesn't lead to a frontrunning attack.

**[H-2]** `AccountForwarder` contract can't receive ETH

There is no way for the `AccountForwarder` contract to receive ETH. Due to this, all the transactions that are executed using the `execute` function will fail if they send ETH to the `caller` contract i.e. `msg.sender`. There is no way to withdraw ETH sent to the contract.

Consider: Creating the` receive` method so that the `AccountForwarder` contract can receive ETH and the transaction that sends ETH to it doesn't revert.

**[M-3]** Use of deprecated packages in import.

On line [4-6], AccountToken.sol has the following code:

import 'openzeppelin-solidity/contracts/token/ERC721/ERC721.sol';
import 'openzeppelin-solidity/contracts/token/ERC721/utils/ERC721Holder.sol';
import 'openzeppelin-solidity/contracts/token/ERC1155/utils/ERC1155Holder.sol';

The package is deprecated as mentioned in the [npmjs](https://www.npmjs.com/package/openzeppelin-solidity). This package is now published as `@openzeppelin/contracts`. And it is suggested change dependency.

Consider: Use `@openzeppelin/contracts` instead of `openzeppelin-solidity`.

**[L-4]** No checks for whether a contract exists when doing an external call in `execute` function.

On line [40], AccountToken.sol has the following code:

return _to.call{value: msg.value}(_data);

There is no check for whether the contract exists at `_to` address. If there is no contract at `_to` address, the external call will succeed and returns `true.`

Consider: Checking if the contract exists at `_to` address before making a call.

**[L-5]** Possibility of NFT to be stuck forever if `AccountForwarder` is deployed from the constructor of the contract.

On line [28-32], AccountToken.sol has the following code:

constructor(address _token)
{
token = AccountToken(_token);
token.safeMint(msg.sender, address(this));
}

The constructor uses `safeMint` which checks that the recipient of the NFT is either `EOA` or a smart contract that can handle NFTs. But if the contract is being deployed from the constructor of the smart contract, the check for `msg.sender` will be considered as `EOA` as there is no code located at the address at that point. If the recipient contract can't handle NFT, it will still be transferred to it.