Streaming Protocol contest - defsec's results

Handle

defsec

Vulnerability details

Impact

function flashLoan in contract Locke.sol does not return a boolean. flashLoan is declared as a function that should return a boolean value, however, in contract Locke there is no return statement so it always gets a default value of false (while base function always returns the opposite, a.k.a true).

Proof of Concept

1. Navigate to the following contract.

https://github.com/code-423n4/2021-11-streaming/blob/main/Streaming/src/Locke.sol

2. Go to the line - https://github.com/code-423n4/2021-11-streaming/blob/main/Streaming/src/Locke.sol#L723

3. Function does not return a value.

Tools Used

None

Recommended Mitigation Steps

It should return super.flashLoan(...).

Handle

defsec

Vulnerability details

Impact

The ecrecover function is used in permit() to recover the address from the signature. The built-in EVM precompile ecrecover is susceptible to signature malleability which could lead to replay attacks (references: https://swcregistry.io/docs/SWC-117, https://swcregistry.io/docs/SWC-121 and https://medium.com/cryptronics/signature-replay-vulnerabilities-in-smart-contracts-3b6f7596df57).

Proof of Concept

https://github.com/code-423n4/2021-11-streaming/blob/main/Streaming/src/LockeERC20.sol#L156

Tools Used

None

Recommended Mitigation Steps

Consider using OpenZeppelin’s ECDSA library (which prevents this malleability) instead of the built-in function.

Handle

defsec

Vulnerability details

Impact

Stream protocol allows different tokens to be used as collateral or underlying. The Dex contracts do not appear to support rebasing/deflationary/inflationary tokens whose balance changes during transfers or over time. The necessary checks include at least verifying the amount of tokens transferred to contracts before and after the actual transfer to infer any fees/interest.

Proof of Concept

Stream whitelists a rebasing/deflationary/inflationary token to be used as collateral or underlying by accident. This leads to miscalculations between internal Pool accounting and the balances in the token contracts.

https://github.com/code-423n4/2021-11-streaming/blob/main/Streaming/src/Locke.sol#L424

Tools Used

Code Review

Recommended Mitigation Steps

• Ensure that to check previos balance/after balance equalts to amount for any rebasing/inflation/deflation
• Add support in contracts for such tokens before accepting user-supplied tokens
• Consider supporting deflationary / rebasing / etc tokens by extra checking the balances before/after or strictly inform your users not to use such tokens if they don't want to lose them.

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 different pragma versions. The contracts pragma version can be updated to 0.8.4 for the gas optimization.

• Affected Contracts

"All Contracts"

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-streaming/blob/main/Streaming/src/Locke.sol#L532

https://github.com/code-423n4/2021-11-streaming/blob/main/Streaming/src/Locke.sol#L516

https://github.com/code-423n4/2021-11-streaming/blob/main/Streaming/src/Locke.sol#L500

https://github.com/code-423n4/2021-11-streaming/blob/main/Streaming/src/Locke.sol#L487

https://github.com/code-423n4/2021-11-streaming/blob/main/Streaming/src/Locke.sol#L455

https://github.com/code-423n4/2021-11-streaming/blob/main/Streaming/src/Locke.sol#L417

https://github.com/code-423n4/2021-11-streaming/blob/main/Streaming/src/Locke.sol#L377

https://github.com/code-423n4/2021-11-streaming/blob/main/Streaming/src/Locke.sol#L365

Tools Used

Slither

Recommended Mitigation Steps

1. Get Smart Contracts from the Repository.
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

For the arithmetic operations that will never over/underflow, using the unchecked directive (Solidity v0.8 has default overflow/underflow checks) can save some gas from the unnecessary internal over/underflow checks.

Proof of Concept

https://github.com/code-423n4/2021-11-streaming/blob/main/Streaming/src/Locke.sol#L360

Tools Used

None

Recommended Mitigation Steps

Consider applying unchecked arithmetic where overflow/underflow is not possible.

Handle

defsec

Vulnerability details

Impact

Various projects (e.g. Uniswap - https://github.com/Uniswap/interface/blob/main/src/hooks/useApproveCallback.ts#L88 , see here 1 using the constant MaxUint256 from ethers.js) set the default value of the user's allowance to 2^256 - 1. Now the value 2^256 - 1 can also be represented in hex as 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff. From Ethereum's yellow paper we know that zeros are cheaper than non-zero values in the hex representation. Considering this fact, an alternative choice could be now 0x8000000000000000000000000000000000000000000000000000000000000000 or 2^255 to represent "infinity". If you do the calculations with Remix, you will see that the former costs 47'872 gas, while the latter costs 45'888 gas. If you accept that infinity can also be represented via 2^255 (instead of 2^256-1) - and I think most projects can live with that - you can already save 1'984 gas (or 4.1%) leveraging this optimisation trick.

Proof of Concept

1. Navigate to the following contract.

"https://github.com/code-423n4/2021-11-streaming/blob/main/Streaming/src/LockeERC20.sol#L113"

Tools Used

Code Review

Recommended Mitigation Steps

Change 2^256-1 With 2^255.

Reference

https://ethereum.github.io/yellowpaper/paper.pdf

https://forum.openzeppelin.com/t/a-collection-of-gas-optimisation-tricks/19966