QA Report

[code423n4]

1. Missing zero address checks

Risk

Low

Impact

Contract Cally does not check for zero addresses which might lead to loss of funds, failed transactions and can break the protocol functionality.

Proof of Concept

• Missing check for token address - https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L160
• Missing check for beneficiary address - https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L351

Tools Used

Manual Review / VSCode

Recommended Mitigation Steps

It is recommended to add zero address checks for listed parameters.

2. Missing events

Risk

Low

Impact

Contract Cally does not implement events for some critical functions. Lack of events makes it difficult for off-chain applications to monitor the protocol.

Proof of Concept

• Missing setFee event - https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L119-L121
• Missing withdrawProtocolFees event - https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L124-L128
• Missing setVaultBeneficiary event - https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L351-L357

Tools Used

Manual Review / VSCode

Recommended Mitigation Steps

It is recommended to add missing events to listed functions.

3. Missing validation for feeRate

Risk

Low

Impact

Function Cally.setFee is missing check for feeRate_ parameter that should not exceed acceptable by protocol percentage.

Proof of Concept

• https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L119-L121

Tools Used

Manual Review / VSCode

Recommended Mitigation Steps

It is recommended to add check for feeRate_ parameter.

4. Owner - critical address change

Risk

Low

Impact

Changing critical addresses such as ownership should be a two-step process where the first transaction (from the old/current address) registers the new address (i.e. grants ownership) and the second transaction (from the new address) replaces the old address with the new one. This gives an opportunity to recover from incorrect addresses mistakenly used in the first step. If not, contract functionality might become inaccessible.

Proof of Concept

• https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L32

Tools Used

Manual Review / VSCode

Recommended Mitigation Steps

It is recommended to implement two-step process for passing ownership.

5. Usage of boolean values

Risk

Non-Critical

Impact

Contract uses false boolean expression for require statements in multiple functions.

Proof of Concept

• https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L217
• https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L220
• https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L328

Tools Used

Manual Review / VSCode

Recommended Mitigation Steps

It is recommended to remove false expression and use alternative syntax such as:

require(!vault.isExercised, "Vault already exercised");

6. Incorrect error message

Risk

Non-Critical

Impact

Function Cally.createVault checks if dutchAuctionReserveStrike is smaller than specified strike option. Incorrect error message "Reserve strike too small" is returned in case of the error. It should be "Reserve strike too big".

require(dutchAuctionReserveStrike < strikeOptions[dutchAuctionStartingStrikeIndex], "Reserve strike too small");

Proof of Concept

• https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L169

Tools Used

Manual Review / VSCode

Recommended Mitigation Steps

It is recommended to change the error message to "Reserve strike too big".

[outdoteth]

this can be bumped to medium severity
3. Missing validation for feeRate: #48

[HardlyDifficult]

Per the C4 guidance "part of auditing is demonstrating proper theory of how an issue could be exploited" and that does not seem to be explored here as it was in the primary report.