Boot Finance contest - defsec's results

Handle

defsec

Vulnerability details

Impact

The burnEtherForMember() functions dont follow check effect interaction pattern. This can cause re-entrancy vulnerability on the function.

Proof of Concept

1. Navigate to the following contract.

"https://github.com/code-423n4/2021-11-bootfinance/blob/main/tge/contracts/PublicSale.sol#L164"

2. The internal state updates should be completed before an external call.

Tools Used

Manual Code Review

Recommended Mitigation Steps

Consider using Openzeppelin's nonReentrant modifier on the related function. More info about the relevant contract implementation can be found here. (https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/security/ReentrancyGuard.sol)

Handle

defsec

Vulnerability details

Impact

During the code review, It has been seen maxVesting amount is disabled. However, there is no maximum and minimum vesting amount defined. Users can vest small amount. For the protocol liquditiy calculation maximum and minimum threshold should be defined.

Proof of Concept

1. Navigate to the following contract.

""" https://github.com/code-423n4/2021-11-bootfinance/blob/main/vesting/contracts/Vesting.sol#L76 """

2. Vesting amount didnt check.

Tools Used

Review

Recommended Mitigation Steps

It is suggested to check maximum/minimum vesting amount on the contract.

Handle

defsec

Vulnerability details

Impact

The addInvestor function which is used on the InvestorDistribution contract does not check duplicate investors. With the malicious admin behaviour, investor can be overwritten.

Proof of Concept

1. Navigate to the following contract.

"https://github.com/code-423n4/2021-11-bootfinance/blob/7c457b2b5ba6b2c887dafdf7428fd577e405d652/vesting/contracts/InvestorDistribution.sol#L85"

2. The contract does not check if the investor exists.

Tools Used

None

Recommended Mitigation Steps

Consider to check if the investor exists before.

Handle

defsec

Vulnerability details

Impact

A comparatively cheap way of storing and reading information is by directly including them into the bytecode of the smart contract, when deploying it on the blockchain. The downside here is that the value cannot be altered afterwards. However, gas consumption for both loading and storing data will be considerably reduced. The array can be marked as constant or immutable that will provide gas optimization. The array is not edited on the contract.

Proof of Concept

1. Navigate to the following the contract.

"https://github.com/code-423n4/2021-11-bootfinance/blob/7c457b2b5ba6b2c887dafdf7428fd577e405d652/vesting/contracts/AirdropDistribution.sol#L23"

2. The contract is using pre-defined array.

Tools Used

None

Recommended Mitigation Steps

Consider to mark array as a constant or immutable for the gas optimization.

Handle

defsec

Vulnerability details

Impact

Using the unchecked keyword to avoid redundant arithmetic underflow/overflow checks to save gas when an underflow/overflow cannot happen. E.g. 'unchecked' can be applied in the following lines of code since there are require statements before to ensure the arithmetic operations would not cause an integer underflow or overflow.

Proof of Concept

1. Review the all contracts and add unchecked keyword where overflow is not possible.

"https://github.com/code-423n4/2021-11-bootfinance/blob/main/vesting/contracts/AirdropDistribution.sol#L579"

Tools Used

Code Review

Recommended Mitigation Steps

Consider applying 'unchecked' keyword where overflows/underflows are not possible.

Handle

defsec

Vulnerability details

Impact

Using newer compiler versions and the optimizer gives gas optimizations and additional safety checks are available for free.

The advantages of versions 0.8.* over <0.8.0 are:

• Safemath by default from 0.8.0 (can be more gas efficient than library based safemath.)
• Low level inliner : from 0.8.2, leads to cheaper runtime gas. Especially relevant when the contract has small functions. For example, OpenZeppelin libraries typically have a lot of small helper functions and if they are not inlined, they cost an additional 20 to 40 gas because of 2 extra jump instructions and additional stack operations needed for function calls.
• Optimizer improvements in packed structs: Before 0.8.3, storing packed structs, in some cases used an additional storage read operation. After EIP-2929, if the slot was already cold, this means unnecessary stack operations and extra deploy time costs. However, if the slot was already warm, this means additional cost of 100 gas alongside the same unnecessary stack operations and extra deploy time costs.
• Custom errors from 0.8.4, leads to cheaper deploy time cost and run time cost. Note: the run time cost is only relevant when the revert condition is met. In short, replace revert strings by custom errors.

Proof of Concept

1. The contest repository contracts contain pragma 0.6.12^. The contracts pragma version should be updated to 0.8.4.

(https://github.com/code-423n4/2021-11-bootfinance/blob/main/customswap/contracts/Swap.sol ) (https://github.com/code-423n4/2021-11-bootfinance/blob/main/customswap/contracts/SwapUtils.sol#L3) (https://github.com/code-423n4/2021-11-bootfinance/blob/main/customswap/contracts/MathUtils.sol#L3) (https://github.com/code-423n4/2021-11-bootfinance/blob/main/tge/contracts/PublicSale.sol#L2) (https://github.com/code-423n4/2021-11-bootfinance/blob/main/tge/contracts/PublicSaleBatchWithdraw.sol#L2)

Tools Used

None

Recommended Mitigation Steps

Consider to upgrade pragma to at least 0.8.4.

Handle

defsec

Vulnerability details

## Impact

This does not directly impact the smart contract in anyway besides cost. This is a gas optimization to reduce cost of smart contract. Calling each function, we can see that the public function uses 496 gas, while the external function uses only 261.

Proof of Concept

According to Slither Analyzer documentation (https://github.com/crytic/slither/wiki/Detector-Documentation#public-function-that-could-be-declared-external), there are functions in the contract that are never called. These functions should be declared as external in order to save gas.

Slither Detector:

external-function:

https://github.com/code-423n4/2021-11-bootfinance/blob/main/tge/contracts/PublicSaleBatchWithdraw.sol#L28

https://github.com/code-423n4/2021-11-bootfinance/blob/main/tge/contracts/PublicSaleBatchWithdraw.sol#L34

Tools Used

Slither

Recommended Mitigation Steps

1. Clone repository for Boot Finance Smart Contracts.
2. Create a python virtual environment with a stable python version.
3. Install Slither Analyzer on the python VEM.
4. Run Slither against all contracts.
5. Define public functions as an external for the gas optimization.

Handle

defsec

Vulnerability details

Impact

From the pragma version 0.8.0, Overflow/underflow protections are enabled by default. The SafeMath library can be deleted from the code base. For achieving gas optimization, unnecessary code should be deleted from the repository.

Proof of Concept

1. Navigate to the following contracts. SafeMath library functions are not used. However It is still exists on the code base.

(https://github.com/code-423n4/2021-11-bootfinance/blob/main/tge/contracts/PublicSale.sol#L22) (https://github.com/code-423n4/2021-11-bootfinance/blob/main/vesting/contracts/Vesting.sol#L20) (https://github.com/code-423n4/2021-11-bootfinance/blob/main/vesting/contracts/InvestorDistribution.sol#L19) (https://github.com/code-423n4/2021-11-bootfinance/blob/main/vesting/contracts/AirdropDistribution.sol#L20)

Tools Used

Code Review

Recommended Mitigation Steps

Consider to delete redundant safe math library for the gas optimization. After pragma 0.8.0, overflow/underflow protections are enabled by default.