Vaporware

The reward system is designed to distribute funds between users that are currently wearing the accessory for which the rewards are distributed. The rewards are generated, when the same accessory is minted for a different `Milady` NFT. But it has been observed that this reward system does not behave as expected and allocates funds in uneven proportions. See the exploit below:

`LiquidAccessories` is an `Ownable` contract that transfers ownership of the contract to `msg.sender`. If this is done via the `Deployer` contract, then the owner of the `LiquidAcessories` contract will be the `Deployer` contract and not the `caller` of the `Deployer` contract. This will result in a situation, where `defineBondingCurveParameter()`, which protected by the `onlyOwner()` modifier, will not be able to be called as the `Deployer` contract does not have any functionality that allows it to call upon this function. This will result in a situation, where no liquid accessories can be minted, since the `bondingCurves` will not be able to be set. This scenario is not captured by the tests, as the tests are using `vm.prank()` to impersonate the address, which in this case will be the `Deployer` contract to test the functionality.

`revenueRecipient` is the address where the whole of `freeRevenue` or half of it is send depending if anyone is wearing that specific accessory or not. It is currently not possible to change this address if the private key of it gets compromised.

Use of the `address.transfer()` function [is discouraged](https://consensys.net/diligence/blog/2019/09/stop-using-soliditys-transfer-now/), since this function forwards only 2300 gas, which might break some non-trivial fallback functions if any future change increases the gas cost of opcodes operations.

It is generally recommended to emit events before external calls. Otherwise, it is possible that events will be emitted in the wrong order if reentrancy occurs. The following events are emitted after external calls. 1. `MiladyAvatar` 1. `AccessoryEquipped` 2. `AccessoryUnequipped` 2. `Rewards` 1. `MiladyDeregisteredForRewards` 2. `RewardsClaimed` 3. `SoulboundAccessories.SoulboundAccessoriesMinted` 4. `TBARegistry.AccountCreated()`

The following validation checks are missing from various contracts. The lack of these checks may lead to the contracts entering an incorrect state. 1. `Deployer.sol` 1. `constructor()` 1. Validate that the string parameters are not empty. 2. Validate that the address parameters are not the zero address. 2. `LiquidAccessories.sol` 1. `mintAccessories()` 1. Validate that the address parameters are not the zero address. 2. Validate that `_accessoryIds`, `_amounts` are not empty arrays. 2. `_mintAccessoryAndDisburseRevenue()` 1. Validate that `_amount` should be non-zero. 3. `burnAccessories()` 1. Validate that the address parameters are not the zero address. 2. Validate that `_minRewardOut` is greater than zero. 4. `_burnAccessory()` 1. `_amount` should be non-zero. 3. `Rewards` 1. `deregisterMiladyForRewardsForAccessoryAndClaim()`: Validate that the address parameters are not the zero address. 2. `claimRewardsForMilady()`: Validate that the address parameters are not the zero address. 4. `SoulboundAccessories` 1. * `mintAndEquipSoulboundAccessories()`: Validate that `_accessories` is not an empty list. 5. `TBARegistry` 1. `createAccount()`: Validate that the address parameters are not the zero address. 6. `TokenGatedAccount` 1. `bond()`: Validate that the address parameters are not the zero address. 2. `execute()`: Validate that the address parameters are not the zero address.

The contract `HardcodedGoerliDeployer` is intended to facilitate testing rather than being deployed as part of the final project. It is ideal to keep production code separate from testing code. A failure to separate them could lead to mistakes in development or the mistaken use of the testing code within production contracts.

There are several functions that transfer value and make numerous external calls. We have identified the following functions that make external calls: 1. `LiquidAccessories` 1. `mintAccessories()` 2. `burnAccessories()` 2. `MiladyAvatar` 1. `updateEquipSlotsByAccessoryIds()` 2. `equipSoulboundAccessories()` 3. `preTransferUnequipById()` 3. `Rewards` 1. `addRewardsForAccessory()` 2. `deregisterMiladyForRewardsForAccessoryAndClaim()` 3. `claimRewardsForMilady()` 4. `SoulboundAccessories.mintAndEquipSoulboundAccessories()` 5. `TBARegistry.createAccount()` 6. `TokenGatedAccount.execute()`

Every Solidity file specifies in the header a version number of the format `pragma solidity (^)0.8.*`. The caret (`^`) before the version number implies an unlocked pragma, meaning that the compiler will use the specified version _and above_, hence the term "unlocked".

As security standards develop, so does the Solidity language. In order to stay up to date with current practices, it's important to use a recent version of Solidity and recent conventions. The version used in all the contracts in scope, `0.8.13`, is known to have vulnerabilities, for more information [see here](https://github.com/ethereum/solidity/blob/11be2f4881d1644f15b026d1022388f06a034a34/docs/bugs_by_version.json#L1738C9-L1738C9).