AdminContract Manual Review Findings

ACT-01M: Insecure Minimum Configurations

Type	Severity	Location
Logical Fault		AdminContract.sol:L224, L272

Description:

The borrowing fee and minimum net debt of the system can be set to 0 which is ill-advised as it can break a lot of economical assumptions about the system as well as render certain types of attacks fiscally viable when borrowing fees are eliminated.

Impact:

The ramifications of zero fees and zero net debt are difficult to pinpoint and thus warrant an unknown severity level for this submission.

Example:

Copy
216/// @inheritdoc IAdminContract
217function setBorrowingFee(
218    address _collateral,
219    uint256 _borrowingFee
220)
221    public
222    override
223    onlyTimelock
224    safeCheck("Borrowing Fee", _collateral, _borrowingFee, 0, 0.1 ether) // 0% - 10%
225{
226    CollateralParams storage collParams = collateralParams[_collateral];
227    uint256 oldBorrowing = collParams.borrowingFee;
228    collParams.borrowingFee = _borrowingFee;
229    emit BorrowingFeeChanged(oldBorrowing, _borrowingFee);
230}

Recommendation:

We advise the system to ensure the minimum permitted values are sensible and non-zero in all configurations of the system.

Alleviation (f6f1ad0b8f):

The Tren Finance team has opted to instead acknowledge this particular exhibit which we advise against due to the fragile nature of fee-less borrowing protocols.

We advise the Tren Finance team to supply us with mathematical proof that the economic model will be able to uphold itself when fees have been disabled as otherwise we cannot consider this exhibit safely acknowledged.

Alleviation (73b9546eb9):

A non-zero minimum net debt and borrowing fee has been imposed in the contract's configurating functions thereby alleviating this exhibit and ensuring the code behaves in line with best practices.

ACT-02M: Inexplicable Capability of Re-Invocation

Type	Severity	Location
Standard Conformity		AdminContract.sol:L144-L146

Description:

The referenced function is meant to be invoked once.

Example:

Copy
144function setSetupIsInitialized() external onlyTimelock {
145    isSetupInitialized = true;
146}

Recommendation:

We advise it to be restricted so, ensuring it cannot be accidentally re-invoked.

Alleviation (f6f1ad0b8f24a96ade345db1dd05a1878eb0f761):

The function prevents re-initialization by evaluating the isSetupInitialized flag, addressing this exhibit.

ACT-03M: Insecure Validation of CCR and MCR Configurations

Type	Severity	Location
Logical Fault		AdminContract.sol:L240, L256

Description:

The system will presently permit the CCR and MCR configurations of an asset to be updated, retroactively affecting all active boxes.

Apart from being insecurely applied immediately, they lack sufficient input sanitization as they permit dangerous values and do not uphold the CCR < MCR relation.

Impact:

The current CCR and MCR maintenance system is insecure and can significantly affect users of the Tren system in an adverse way.

Example:

Copy
232/// @inheritdoc IAdminContract
233function setCCR(
234    address _collateral,
235    uint256 _newCCR
236)
237    public
238    override
239    onlyTimelock
240    safeCheck("CCR", _collateral, _newCCR, 1 ether, 10 ether) // 100% - 1,000%
241{
242    CollateralParams storage collParams = collateralParams[_collateral];
243    uint256 oldCCR = collParams.ccr;
244    collParams.ccr = _newCCR;
245    emit CCRChanged(oldCCR, _newCCR);
246}
247
248/// @inheritdoc IAdminContract
249function setMCR(
250    address _collateral,
251    uint256 _newMCR
252)
253    public
254    override
255    onlyTimelock
256    safeCheck("MCR", _collateral, _newMCR, 1.01 ether, 10 ether) // 101% - 1,000%
257{
258    CollateralParams storage collParams = collateralParams[_collateral];
259    uint256 oldMCR = collParams.mcr;
260    collParams.mcr = _newMCR;
261    emit MCRChanged(oldMCR, _newMCR);
262}

Recommendation:

We advise the system to refactor how CCR and MCR adjustments take place. The CCR and MCR configurations of an asset should have an adequate minimum delta between them to permit sufficient time for borrowers to react to a negative market event.

The CCR and MCR configurations should never be permitted so close to 100% (presently 100% and 101%) as it would render liquidation of positions to always result in bad debt.

Both configuration functions should ensure that the CCR configured is less than the MCR by the buffer we advised in our first recommendation for the system.

Finally, adjustments to the CCR and MCR should be gradually applied either after a time threshold elapses permitting adequate time for users to render their boxes healthy, or using a time-based function that gradually scales each collateral ratio to the new value.

Alleviation (f6f1ad0b8f):

A gradual adjustment system has been introduced for the CCR and MCR configurations of the system, however, the critical CCR < MCR relation is not upheld and the gradual system has introduced a new flaw.

Specifically, the gradual update system is optional when updating the MCR and CCR configurations of an asset. This permits a second update performed while the first one is still gradually being resolved to abide by the gradual release regardless of the _applyGracePeriodDeadline flag.

This flaw arises from the fact that the mcrUpdateDeadline value is not cleared when _applyGracePeriodDeadline is set to false and should be resolved.

Alleviation (73b9546eb9):

The code was updated to ensure a new configuration has a delta of at least 0.4 ether between the MCR and CCR values, however, it does not resolve the newly introduced vulnerability outlined in the previous alleviation chapter.

Alleviation (13f0ca88ab):

The gradual adjustment system has been removed from the codebase opting for a time-based update mechanism with a maximum delta of 0.1 ether, thereby ensuring that the update of the MCR and CCR values is not immediately reflected in the system and is instead applied after a time delay.

We consider the original as well as the newly introduced issues to be resolved in the latest implementation.