Foodriver security record

The rates for the private and public sale, privateSaleRate and publicSaleRate describe a 100 to 1 rate to support floating numbers. When these values get set in the updateTokenRates function it allows the following require checks to pass require(supportedTokens[token].privateSaleRate > 0, "Unsupported token or rate is 0"); require(supportedTokens[token].publicSaleRate > 0, "Unsupported token or rate is 0"); However it doesn't make the payout calculations correct.

Tokens that comply with the ERC20 standard are expected to: • Moves amount tokens from the caller’s account to recipient. • Returns a boolean value indicating whether the operation succeeded.  • Emits a Transfer event. After an ERC20 token transfer, it is expected that the 3 conditions above are satisfied. It is recommended to validate the transfers with a check on the boolean value returned as some tokens do not fully comply with this specification (commonly called weird-erc20-tokens). If there is no validation on the return value of the transfer or transferFrom calls, the remainder of the function could run to completion without the tokens being transferred.   The purchasePublic and purchasePrivate functions call transferFrom on the token address passed and are vulnerable to this attack - if the function calls fail or revert for any reason (such as insufficient token balances) tokens will still be minted. Users will be able to effectively game the protocol by minting tokens in either of these sale periods without paying out those tokens.

Upgradeable contracts allow for smart contracts to have extended functionality. They can be deployed as is, and upgraded at a later time to adjust the implementation. Within composable upgradeable contracts, the issue of storage collision exists which could alter the reading of storage for each of the contracts built atop each other.To avoid issues when reading or writing to storage it is advisable to have a gap introduced in each contract to prevent the new storage variables added from crossing into existing storage locations. Here, OpenZeppelin describes in detail how to manage smart contract storage with gaps: Writing Upgradeable Contracts - OpenZeppelin Docs

For orders created by the Store contract via the registry, there is no input validation for the different percentages that are passed in as arguments. It is expected that these values should not exceed 100% of the total amount but not enforced in the codebase. This would make some functions to revert or continue to run till the end and fail to transfer the expected token balances to the respective parties.

The Check-Effects-Interact [CEI] pattern helps to greatly reduce the risk of reentrancy in contracts, it is a best practice to emit events as part of the Effects before any external calls or interactions happen which could open up a reentrancy vulnerability.

The ERC20 internal _mint function does not revert when 0 tokens are passed in as an argument. A transfer event will also be emitted for the 0 token mints which would be unnecessary, especially for on-chain monitoring software.

The current implementation of the token sale does not have any timing schedules for users to work with. Some of the issues with this are:• The public and private sale could happen concurrently, and this would cause some concern because public sales usually happen after the private sale is ended or fully subscribed to. • There is also no duration given for the tokens to be bought within, both sales could happen in a single hour not allowing the community to fully participate.

The comment * @notice Retrieves the address of a store by its ID. does not describe accurately what the function returns. The return value is a specific interface for the FooDriverStore, not an address as the comment says.