OriginTrail security record

The OT-RFT10 document describes something similar to a finite state machine (FSM) with at least 4 states, i.e. boarding period, starfleet launch, bridge launch, fallback period. However it is unclear what the transition conditions between these states are. For instance, the table on page 4 of the OT-RFT10 document indicates that in addition to the timestamp constraint of each of the periods, there is an additional condition for transitioning from the "boarding period" state to the "starfleet launch" state, namely the `MIN_THRESHOLD` must be reached. However, there is no functional requirement that indicates that the starfleet launch cannot start if the `MIN_THRESHOLD` is not reached. Therefore, the specification is not clear for the transition from the "boarding period" state to the "starfleet launch" state. <br><br> In the implementation this unclarity translates to a violation of FR3 which states that: > "FR3 Token holders SHALL NOT be able to withdraw TRAC tokens during the Starfleet launch window and Bridge launch window phases." This leads to the possibility of users withdrawing their staked deposits within the starfleet launch and bridge launch windows in case the `MIN_THRESHOLD` is not reached.

A Denial-of-Service (DoS) attack is a situation which an attacker renders a smart contract unusable and/or when the contract owner is forced to call the contract too many times, e.g.: 1. If the number of participants is high enough, the `accountStarTRAC` function may fail due to an out of gas error, which would mean that the input array of `contributors` and `amounts` needs to be partitioned and the functions needs to be called multiple times for each partition 2. If one has enough funds to make many small deposits, accounting as performed by `accountStarTRAC` could require more gas than the limit set within a block. In the latter case, accounting will require breaking the `contributors` array into smaller chunks, and one will have to send one transaction per chunk, which could be very costly and time consuming. Note that this is related to the 3rd paragraph of QSP-2. Since recommendations 1 and 3 were not implemented, the issue described here is possible.

The `accountStarTRAC` function assumes that all entries in the `contributors` array are distinct. While that is a fair assumption, it should be documented and enforced in the code.

Solidity 0.7.0 deprecated the `now` keyword. For contracts ^0.7.0, you must use `block.timestamp`. Since the pragma of the contract is unlocked and allows versions between 0.6.0 and 0.8.0 this could be an important issue.

Every Solidity file specifies in the header a version number of the format `pragma solidity (^)0.6.*` or `pragma solidity >=0.6.0 <=0.8.0`. The caret (`^`) or the greater/less than or equal (`>=`, `<=`) before the version number implies an unlocked pragma, meaning that the compiler will use the specified version _and above_, hence the term "unlocked". Given that only certain versions of the Solidity compiler are recommended for production use it is dangerous to use this wide range of versions in the pragma. Moreover, the compiled contracts with one version of the compiler might behave slightly differently than when compiled with a different version of the compiler.

`.transfer()` forwards exactly 2,300 gas to the recipient. The goal of this hardcoded gas stipend was to prevent reentrancy vulnerabilities, but this only makes sense under the assumption that gas costs are constant. Recently EIP 1884 was included in the Istanbul hard fork. One of the changes included in EIP 1884 is an increase to the gas cost of the SLOAD operation, causing a contract's fallback function to cost more than 2300 gas.