# MAD Audit
## Info
- Auditor: Mohamed Boukri.
- Commit hash:
git repository (commit hash)
## Disclaimers
## Issues
## MADRouter721.sol
### Value Verification [MEDIUM]
**Description**
The collection creator can set any value in fee variable. This represent a big risk on the user side.
**File(s)**
* `contracts/MADRouter721.sol` (L352)
**Recommendation**
We recommend to limit the fee value by adding a require statement.
### Missing address verification [MEDIUM]
**Description**
The address-type arguments newOwner and _signer should include a zero-address test, otherwise, the contract's functionality may become inaccessible. If the contract ownership is lost. You need to re-deploy the same contract again.
**File(s)**
* `contracts/MADRouter721.sol` (L426,L447)
**Recommendation**
We recommend that you make sure the addresses provided in the arguments are different from the address(0) by adding a require statement.
## MADRouter1155.sol
### Value Verification [Medium]
**Description**
The collection creator can set any value in fee variable. This represent a big risk on the user side.
**File(s)**
* `contracts/MADRouter1155.sol` (L403)
**Recommendation**
We recommend to limit the fee value by adding a require statement.
### Missing address verification [Medium]
**Description**
The address-type arguments newOwner and _signer should include a zero-address test, otherwise, the contract's functionality may become inaccessible. If the contract ownership is lost. You need to re-deploy the same contract again.
**File(s)**
* `contracts/MADRouter1155.sol` (L480,L496)
**Recommendation**
We recommend that you make sure the addresses provided in the arguments are different from the address(0).
## MADMarketplace721
### Value Verification [Medium]
**Description**
The owner can set any value in fee variable. This represent a big risk on the user side.
**File(s)**
* `contracts/MADMarketplace721.sol` (L381)
**Recommendation**
We recommend to limit feeVal2 and feeVal3 values by adding a require statement.
### Race Condition [MEDIUM]
**Description**
The feeVal2 , feeVal3 variables have a setter. If the user checks the value of this variable, then calls the buy or call function, and the owner updates the Fees Value , the order of the transaction might overturn and the user’s transaction in this case will be executed with the new fees without him knowing about it.
**File(s)**
* `contracts/MADMarketplace721.sol` (L381)
**Recommendation**
Consider adding feeVal2 feeVal3 variables in the arguments of the function then add require statements that verifies that the value provided in the arguments is the same as the one that is stored in the smart contract.
### Missing address verification [MEDIUM]
**Description**
The address-type arguments newOwner and _recipient should include a zero-address test, otherwise, the contract's functionality may become inaccessible. If the contract ownership is lost. You need to re-deploy the same contract again.
**File(s)**
* `contracts/MADRouter721.sol` (L441,L454)
**Recommendation**
We recommend that you make sure the addresses provided in the arguments are different from the address(0).
## MADMarketplace1155
### Missing address verification [Medium]
**Description**
The address-type arguments newOwner and _recipient should include a zero-address test, otherwise, the contract's functionality may become inaccessible. If the contract ownership is lost. You need to re-deploy the same contract again.
**File(s)**
* `contracts/MADRouter1155.sol` (L466,L479)
**Recommendation**
We recommend that you make sure the addresses provided in the arguments are different from the address(0).
### Value Verification [Medium]
**Description**
The owner can set any value in fee variable. This represent a big risk on the user side.
**File(s)**
* `contracts/MADMarketplace1155.sol` (L405)
**Recommendation**
We recommend to limit feeVal2 and feeVal3 values by adding a require statement.
## MADFactory721
### Missing address verification [Medium]
**Description**
The address-type arguments newOwner _market _router _signer should include a zero-address test, otherwise, the contract's functionality may become inaccessible. If the contract ownership is lost. You need to re-deploy the same contract again.
**File(s)**
* `contracts/MADRouter721.sol` (L487,L502,L512,L523)
**Recommendation**
We recommend that you make sure the addresses provided in the arguments are different from the address(0).
### For Loop Over Dynamic Array [LOW]
**Description**
When smart contracts are deployed or their associated functions are invoked, the execution of these operations always consumes a certain quantity of gas, according to the amount of computation required to accomplish them.
Modifying an unknown-size variable that grows over time can result in a Denial Of Service. Simply by having an excessively large array, users can exceed the gas limit, therefore preventing the transaction from ever succeding.
**File(s)**
* `/contracts/MADFactory721.sol` (L729)
Recommendation:
We recommend to avoid any actions that involve looping across the entire data structure. if you really must loop over an array of unknown size, you will need to arrange for it to consume many blocks and thus multiple transaction.
## MADFactory1155
### Missing address verification [Medium]
**Description**
The address-type arguments newOwner _market _router _signer should include a zero-address test, otherwise, the contract's functionality may become inaccessible. If the contract ownership is lost. You need to re-deploy the same contract again.
**File(s)**
* `contracts/MADRouter1155.sol` (L480,L495,L505,L516)
**Recommendation**
We recommend that you make sure the addresses provided in the arguments are different from the address(0).
### For Loop Over Dynamic Array [LOW]
**Description**
When smart contracts are deployed or their associated functions are invoked, the execution of these operations always consumes a certain quantity of gas, according to the amount of computation required to accomplish them.
Modifying an unknown-size variable that grows over time can result in a Denial Of Service. Simply by having an excessively large array, users can exceed the gas limit, therefore preventing the transaction from ever succeding.
**File(s)**
* `/contracts/MADFactory1155.sol` (L721)
Recommendation:
We recommend to avoid any actions that involve looping across the entire data structure. if you really must loop over an array of unknown size, you will need to arrange for it to consume many blocks and thus multiple transaction, or limit token creation.
## ERC20.sol
Approve Race Condition [LOW]
Description:
The standard ERC20 implementation contains a widely known racing condition in its
approve function, wherein a spender can witness the token owner broadcast a transaction
altering their approval and quickly sign and broadcast a transaction using transferFrom to
move the current approved amount from the owner’s balance to the spender. If the
spender’s transaction is validated before the owner’s, the spender will be able to get both
approval amounts of both transactions.
Risk Level:
Likelihood – 1
Impact - 2
Recommendation:
We recommend using increaseAllowance and decreaseAllowance functions to modify the
approval amount instead of using the approve function to modify it.
Missing Value Verification can lead to ruin contract logic [Medium]
Description
Certain functions lack a safety check in the values, the values of the arguments should be verified to allow only the ones that go with the contract’s logic.The variables in UpdateSettings() function should be verified otherwise, the user can’t create an order , bid, or the auction won’t increment.
File(s)
contracts/MADMarketplace721.sol (L403)
Recommendation
consider verifying _minAuctionIncrement _minOrderDuration and _minBidValue by adding a require statement, to allow only the ones that go with the contracts’s logic