Jake's Audit Notes: Firestarter

# Jake's Audit Notes: Firestarter Client: Firestarter https://www.firestarter.fi/ https://github.com/Firestarter-Finance/contracts main/`eb96d7b` ## Whitepaper & specification about the project Separate documentation provided declaring intent and implementation. Project is a funding platform for initial metaverse offerings (IMO), and has whitelisting, lock up periods, and vesting schedules. The native token is FLAME. ## Review of the protocol/implementation [1] **Unlocked Pragma** **Files Affected:** `CustomToken.sol`, `FirestarterPresale.sol`, `Presale.sol`, `ProjectPresale.sol`, `TokenLock.sol`, `Vesting.sol`, `Whitelist.sol`. **Severity: Low** The version of the Solidity defined in the code is not fixed. This can lead to unintended or unexpected behaviours in the implementation due to different compiler versions between what is intented in the code and the output at compile time. **Recommendations:** [a] Statically define the version of the Solidity compiler using `pragma solidity 0.8.0`, where the desired release of the `0.8` train is denoted. [2] **Old Semantics Used With Version `0.8`** **Files Affected:** `TokenLock.sol`, `FirestarterPresale.sol`, `Whitelist.sol`, `Presale.sol`, `ProjectPresale.sol`, `Vesting.sol`, `Staking.sol` **Severity: Informational** The `SafeMath` library is enabled by default with `0.8` releases of Solidity and is not required to be imported as over/underflows will be reverted. The ABI decoder statement is valid but deprecated and therefore has no effect. **Recommendations:** [a] Remove the `SafeMath` and `SignedSafeMath` library imports, and associated `using SafeMath for uint256;` statements. [b] Change the ABI coder statements from `pragma abicoder v2;` -> `pragma abicoder v2;` Further information: https://docs.soliditylang.org/en/v0.8.7/080-breaking-changes.html [3] **Unprotected Constructor** **Severity: High** The [`initialize()`](https://github.com/Firestarter-Finance/contracts/blob/eb96d7b6e48d93be79832392953d8e00fc0c27bd/contracts/Presale.sol#L187) function is able to be called by anybody post deployment, which would allow an unauthorised actor to assume the `DEFAULT_ADMIN_ROLE`. Also applies to [`Vesting.sol/initialize()`](https://github.com/Firestarter-Finance/contracts/blob/eb96d7b6e48d93be79832392953d8e00fc0c27bd/contracts/Vesting.sol#L104) and [`Staking.sol/initialize`](https://github.com/Firestarter-Finance/contracts/blob/eb96d7b6e48d93be79832392953d8e00fc0c27bd/contracts/Staking.sol#L88). [a] Either employ a constructor with `constructor()` , or protect the `initialize()` function with a modifier. ### Presale.sol [1] **Partial `goalFunds` implementation** **Severity: Low** The struct [`PresaleParams`](https://github.com/Firestarter-Finance/contracts/blob/main/contracts/Presale.sol#L41) implements the member `goalFunds`. Access to this is commented out when accessed via `goalFunds = _presale.goalFunds;`. **Recommendations:** [a] Please clarify the intention of this feature. Either implement `goalFunds` or remove the partial implementation from the project. ### Vesting.sol [1] **Change of Owner Missing Event** **Severity: Low** [`init()`](https://github.com/Firestarter-Finance/contracts/blob/eb96d7b6e48d93be79832392953d8e00fc0c27bd/contracts/Vesting.sol#L138) allows the change of the owner to be the pre-sale contract. An event should be emitted when this occurs. **Recommendations:** [a] Emit and event for production monitoring of the change of owner. **Recommendations:** [a] Either employ a constructor with the `constructor()` keyword, or protect the `initialize()` function with an access modifier. ### Staking.sol [1] **Contract Could Be Left Without Ownership** **Severity: Medium** [`renounceOwnership()`](https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable/blob/master/contracts/access/OwnableUpgradeable.sol#L59) from OpzenZeppelin allows an actor with the `onlyOwner` role to rennounce the ownership of the contract by setting the owner to the zero address. This would prevent any further functions with `onlyOwner` to be callable. **Recommendations:** [a] Override `renounceOwnership()` in the contract with a `revert()` so that the OZ library can be used without the renouncement of the ownership.