Covalent contest - defsec's results

Handle

defsec

Vulnerability details

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:

DelegatedStaking.sol

https://github.com/code-423n4/2021-10-covalent/blob/main/contracts/DelegatedStaking.sol#L90
https://github.com/code-423n4/2021-10-covalent/blob/main/contracts/DelegatedStaking.sol#L101
https://github.com/code-423n4/2021-10-covalent/blob/main/contracts/DelegatedStaking.sol#L214
https://github.com/code-423n4/2021-10-covalent/blob/main/contracts/DelegatedStaking.sol#L257
https://github.com/code-423n4/2021-10-covalent/blob/main/contracts/DelegatedStaking.sol#L269
https://github.com/code-423n4/2021-10-covalent/blob/main/contracts/DelegatedStaking.sol#L304
https://github.com/code-423n4/2021-10-covalent/blob/main/contracts/DelegatedStaking.sol#L345
https://github.com/code-423n4/2021-10-covalent/blob/main/contracts/DelegatedStaking.sol#L355
https://github.com/code-423n4/2021-10-covalent/blob/main/contracts/DelegatedStaking.sol#L368
https://github.com/code-423n4/2021-10-covalent/blob/main/contracts/DelegatedStaking.sol#L386
https://github.com/code-423n4/2021-10-covalent/blob/main/contracts/DelegatedStaking.sol#L393
https://github.com/code-423n4/2021-10-covalent/blob/main/contracts/DelegatedStaking.sol#L409
https://github.com/code-423n4/2021-10-covalent/blob/main/contracts/DelegatedStaking.sol#L402

Tools Used

Slither

Recommended Mitigation Steps

1. Clone repository for Covalent Smart Contract.
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

It is good to add a require() statement that checks the return value of token transfers or to use something like OpenZeppelin’s safeTransfer/safeTransferFrom unless one is sure the given token reverts in case of a failure. Failure to do so will cause silent failures of transfers and affect token accounting in contract.

Reference: This similar medium-severity finding from Consensys Diligence Audit of Fei Protocol: https://consensys.net/diligence/audits/2021/01/fei-protocol/#unchecked-return-value-for-iweth-transfer-call

Proof of Concept

1. Navigate to https://github.com/code-423n4/2021-10-covalent/blob/main/contracts/DelegatedStaking.sol#L77 contract.
2. Transfer and TransferFrom function is used instead of safe transfer on the following lines.

https://github.com/code-423n4/2021-10-covalent/blob/main/contracts/DelegatedStaking.sol#L77 https://github.com/code-423n4/2021-10-covalent/blob/main/contracts/DelegatedStaking.sol#L85

Tools Used

Manual Code Review

Recommended Mitigation Steps

Consider using safeTransfer/safeTransferFrom or require() consistently.