VaultBase Manual Review Findings

VBE-01M: Unknown Integration Points

Type	Severity	Location
External Call Validation		VaultBase.sol:L196, L204, L323

Description:

The referenced statements represent integration points with an IBalancer contract that is not in scope of this engagement.

Example:

Copy
192if (deposit > targetDeposit) {
193  uint256 amount = deposit - targetDeposit;
194
195  _s.asset.safeTransfer(address(_s.balancer), amount);
196  _s.balancer.deposit(_s.asset, user, amount);
197
198  info.deposit -= amount;
199}
200
201if (yield > targetYield) {
202  uint256 amount = yield - targetYield;
203
204  amount = _s.balancer.withdraw(_s.asset, user, amount);
205  info.deposit += amount;
206}

Recommendation:

We advise the 0xPhase team to adequately validate the referenced integration points and to potentially include the contract in scope of the audit if it comprises part of the 0xPhase protocol's internal architecture.

Alleviation (3dd3d7bf0c2693b2f9c23bacedfa420393f7ea84):

The implementation of the balancer has been audited in full, rendering this exhibit alleviated.

VBE-02M: Dangerous Order of Mathematical Operations

Type	Severity	Location
Mathematical Operations		VaultBase.sol:L284-L285

Description:

The referenced mathematical operations represent significant accuracy numbers that are prone to overflow if they are significantly large before being normalized via the ensuing division.

Impact:

Significantly high collateral ratios, price reports as well as deposit amounts can cause the VaultBase::_depositValue function to revert which is an integral function to the vault's correct operation.

Example:

Copy
277/// @notice Returns the deposit value of the amount in dollars
278/// @param amount The collateral amount
279/// @return The deposit value
280function _depositValue(uint256 amount) internal view returns (uint256) {
281  if (amount == 0) return 0;
282
283  return
284    _scaleFromAsset(_price() * amount * _s.maxCollateralRatio) /
285    (1 ether * 1 ether);
286}

Recommendation:

We advise the calculations to be performed in pairs, invoking VaultBase::_scaleFromAsset solely on the amount argument, multiplying the result with VaultBase::_price, dividing by the 1 ether value literal, multiplying by _s.maxCollateralRatio, and finally dividing by 1 ether again, ensuring the interim calculations are within proper arithmetic bounds and do not cause an unintended overflow to render VaultBase::_depositValue inoperable.

Alleviation (3dd3d7bf0c2693b2f9c23bacedfa420393f7ea84):

All calculations have been split as advised, ensuring maximum accuracy whilst performing all calculations within acceptable bounds.

VBE-03M: Incorrect Definition of Diamond Storage

Type	Severity	Location
Logical Fault		VaultBase.sol:L27

Description:

The VaultBase contract declares its storage as a contract-level variable rather than a dynamically loaded storage offset.

Impact:

The current storage utilization method is non-standard and can cause significant corruption if misused as it is inherently incompatible with the Diamond standard.

Example:

Copy
23abstract contract VaultBase is OwnableBase, AccessControlBase {
24  using EnumerableSet for EnumerableSet.AddressSet;
25  using SafeERC20 for IERC20;
26
27  VaultStorage internal _s;

Recommendation:

We advise a dynamically loaded storage offset to be utilized akin to other facet implementations present in the diamond directory of the repository, ensuring that no storage collisions can occur.

Alleviation (3dd3d7bf0c2693b2f9c23bacedfa420393f7ea84):

The storage slot is dynamically loaded in compliance with the Diamond standard by utilizing a slot offset and a dynamic VaultBase::_s function.