# Quant Protocol - Halborn Report Response

### (HAL-01) Vulnerable TimelockController Contract

As recommended by Halborn, we updated the TimelockController contract to use the revised timelock contract from version 3.4.2-solc-0.7 of the OpenZepellin contracts, which included a fix to the vulnerability reported by Halborn.

The changes were added at the following commit: [1550fd9](https://github.com/Quant-Finance-HQ/quant-protocol/commit/1550fd902cbce5ba440d7605b8b84f91a7eba986).

### (HAL-02) Misuse of an Oracle

We acknowledge and understand the recommendation given by Halborn to check if the latest price data returned by a Chainlink oracle is recent. Still, since that method is only a utility to be used by our app's UI, we decided not to add the suggested checks because we want the UI always to get some price data to display, even if it's a bit stale.

In conclusion, the reported issue doesn't affect any critical contract functionality.

### (HAL-03) Lack of Integer Overflow Protection

As pointed out by the auditors, we added overflow and underflow protection to all the arithmetic operations missing that.

We added those changes in this commit: [6e70b46](https://github.com/Quant-Finance-HQ/quant-protocol/pull/62/commits/6e70b46e8ac082bd42d5b39e42ad51db5fc46cf1).

### (HAL-04) Lack of Maximum Amount Check on the Exercise Flow

Suppose a user calls the exercise action on the controller with an amount argument greater than his QToken balance. In that case, we want that call to revert instead of succeeding by exercising the whole user's balance.

But when a user calls exercise with 0 as the amount, the controller exercises the total user's balance for the given QToken. We added that as a feature, so users don't need to get their balance before exercising.

### (HAL-05) Weak Referral Code Generation

We didn't intend for referral codes to use cryptographically secure random numbers. We just wanted them to be the first 32 bytes of a valid string. The [`validateCode`](https://github.com/Quant-Finance-HQ/quant-protocol/blob/8ff1eed3742b0739966200d5ee8ad484d89468ff/contracts/libraries/ReferralCodeValidator.sol#L13) function only checks if a given string is less than 32 characters long, non-empty, doesn't start with 0x, and contains only digits(0-9) and uppercase and lowercase characters from A-Z or a-z.

### (HAL-06) Unchecked Transfer

We fixed this issue on the following commit: [96fd197](https://github.com/Quant-Finance-HQ/quant-protocol/pull/62/commits/96fd197eec5f0c5738b841dced1246d689a6da6e).

### (HAL-07) Oracle is not activated during the first installation

Even though it would save the oracle manager some gas, as indicated by the auditors, we decided to keep the oracle addition and activation on the registry as two separate methods and oracle management steps. In other words, we want newly added oracles to be inactive by default.

### (HAL-08) Front-runnable Initializers

As per the OpenZeppelin upgradable contracts package, upgradable contracts can only have a single method with the `initializer` modifier (usually called `initialize`) and not a constructor. The `initializer` modifier checks and guarantees that the `initialize` method is only called once during deployment, and the `msg.sender` in the context of that call is the deployer. Thus, the initialize method in our upgradeable contracts can not be front-run.

### (HAL-09) Floating Pragma

All contracts are now fixed to a 0.7.6 version pragma per the following commit: [594ecaf](https://github.com/Quant-Finance-HQ/quant-protocol/pull/62/commits/594ecafd46edb77d513fe5acc6ea57a03164b8fe).

The only files left unchanged were the interfaces for possible integration and interoperability reasons.

### (HAL-10) Lack of Zero Address Check

Missing zero address checks were added in the commit with hash [737169d](https://github.com/Quant-Finance-HQ/quant-protocol/pull/62/commits/737169d664c30c316e8c0c7f982b240a128f9dcc).

### (HAL-11) Ignored return values

When minting spreads, we now check that the external call to `collateralToken.createCollateralToken` succeeds and returns the correct id: [b045abb](https://github.com/Quant-Finance-HQ/quant-protocol/pull/62/commits/b045abb2c6850e96b4605101357fc0a291e2f6cc).

### (HAL-12) Use of block.timestamp

After further analysis, we determined that there's no risk of MEV attacks regarding the places where we used `block.timestamp`. Even if a miner were to manipulate the block timestamp to an earlier time, there would still need to be a price at the option expiration in the Chainlink oracle before an attacker could exercise the option.

### (HAL-13) Use of inline assembly

We recognize where issues with inline assembly could arise. Still, in the case of our protocol contracts, we're only using it for manipulating bytes in memory where there are no critical protocol operations involved and no risk of malicious manipulation by users.

### (HAL-14) Experimental features enabled (ABIEncoderV2)

We checked and validated that the solidity version we're using (0.7.6) already includes fixes to the bug mentioned. The relevant fixes were added in Solidity v0.5.7: https://github.com/ethereum/solidity/releases/tag/v0.5.7.

### (HAL-15) External calls within a loop

The worse that could happen is attackers running out of gas when trying to execute too many actions through the Controller. But for regular users and expected calls to the `operate` method, the loop over specified actions and their external calls shouldn't be a problem.

### (HAL-16) Uninitialized function variable

Fixed in [5272331](https://github.com/Quant-Finance-HQ/quant-protocol/commit/52723315a57441e505069921eed2103764c8919e).

### (HAL-17) For-loop over dynamic array

The functions in the ConfigTimelockController that loop over user-provided dynamic arrays can only be called by protocol multisig proposers and executors. So, in this case, there's no room for external and malicious DoS attempts.