Carry security record

As it presently is constructed, the contract is vulnerable to the allowance double-spend exploit, as with other ERC20 tokens. An example of an exploit goes as follows: 1. Alice allows Bob to transfer N amount of Alice's tokens (N>0) by the calling approved() method on Token smart contract (passing Bob's address and N as method arguments) 2. After some time, Alice decides to change from N to M (M>0) the number of Alice's tokens Bob is allowed to transfer, so she calls the approve() method again, this time passing Bob's address and M as method arguments 3. Bob notices Alice's second transaction before it was mined and quickly sends another transaction that calls the transferFrom() method to transfer Alice's tokens somewhere 4. If Bob's transaction will be executed before Alice's transaction, then Bob will successfully transfer N Alice's tokens and will gain an ability to transfer another M tokens 5. Before Alice notices any irregularities, Bob calls transferFrom() method again, this time to transfer M Alice's tokens. The exploit (as described above) is mitigated through use of functions that increase/decrease the allowance relative to its current value, such as increaseAllowance and decreaseAllowance. Pending community agreement on an ERC standard that would protect against this exploit, we recommend that developers of applications dependent on approve()/transferFrom() should keep in mind that they have to set allowance to 0 first and verify if it was used before setting the new value. Teams who decide to wait for such a standard should make these recommendations to app developers who work with their token contract.

The client has confirmed that the presale has already concluded and this contract is no longer relevant. As mentioned in the previous audit, _processPurchase() still has a vulnerability where a beneficiary can be added multiple times, compromising the gradual delivery of tokens, as given by functions deliverTokensInRatio() and deliverTokensInRatioFromTo(). Exploit Scenario: To illustrate how this could happen, consider the following execution flow: 1. Bob buys 100 CRE tokens. Thus, Bob is added as a beneficiary in the beneficiaries array at position k' = beneficiaries.length. His balance is now 100 CRE. 2. Bob is refunded 100 CRE tokens. His balance is now 0. 3. Bob rebuys 100 CRE tokens. Again, Bob is added as a beneficiary in the beneficiaries array at a position k'' = beneficiaries.length and k'' > k'. 4. The contract Owner now wishes to deliver 1/2 of the tokens. As such, Owner calls deliverTokensInRatio(1, 2). The expectation here is that Bob should receive 50 CRE. 5. Function deliverTokensInRatio() executes, leading to a call to _deliverTokensInRatio(1, 2, 0, beneficiaries.length). 6. The execution of _deliverTokensInRatio() iterates over the beneficiaries array. Delivering tokens to Bob leads to: 1. When i = k', Bob receives 1/2 of his 100 balance, i.e., 50 CRE. His remaining balance is now 100 - 50 = 50. 2. When i = k, the contract will again deliver tokens to Bob. Specifically, Bob receives 1/2 of his 50 balance, i.e., 25 CRE. His remaining balance is 50 - 25 = 25. As shown from the flow, Bob receives 75 CRE, instead of the expected 50.

The following is a list of code defects that are bugs, do not adhere to Solidity best practices, affect gas use, or are deprecated/obsolete. CarryPublicTokenCrowdsale.sol [Update: confirmed fix] In the constructor, it may be easier to understand and less prone to human error if _individualMaxCaps is split up into two arrays of equal length instead of passing in a flattened array of tuples. The check require(_individualMaxCaps.length % 2 == 0) would have to be replaced with a require statement that checks the two arrays are of equal length. Furthermore, the owner should make sure timestamps are not duplicated. [Update: confirmed fix] L156 Having to loop through the entire individualMaxCaps is not optimal in terms of gas cost. It is recommended to include sorted list validation during contract construction. This way, there’s no need to look through all members of the array. Furthermore, we can do the following to optimize searching through the array. To eliminate old members of the array: pop off members of the array as you go along. If it’s past the time for some individualMaxCap now, it always will be in every future iteration. To avoid looking at new members of the array, choose one of the following criteria: Solution 1: Break at the first timestamp value greater than block.timestamp, and save the previous value. Issue: Still vulnerable to block time manipulation. However, if a 90-second deviation can be tolerated, then the issue is practically non-existent. Solution 2: Break at the first timestamp value greater than latestTimestamp. Save the index for this value in storage and start from there in future iterations. Issue: Burden of gas will be higher on some users than others, but it will be less for everyone overall. L221 super._updatePurchasingState(_beneficiary, _weiAmount); is unnecessary as it isn’t implemented in any of the base classes. L227 The for loop in addAddressesToWhitelist may run out of gas if the length of _beneficiaries is too big. L260 Rename function _deliverTokens(address _beneficiary) internal._deliverTokens(_beneficiary, amount) is already used in the standard Crowdsale.sol contract, and while the arguments may differ, using the same function name will be a source of unnecessary confusion.