Omniscia Olympus DAO Audit

BondDepository Manual Review Findings

BondDepository Manual Review Findings

BDY-01M: Improper Bond Price Assumption

Description:

The _bondPrice adjusts the on-chain minimum price of a bond to 0 if the price of the bond is exceeded, however, the debtRatio variable relies on the total supply of OHM which is fully manipulateable and thus can cause the price of a bond to increase above the minimum temporarily by taking advantage of the debtRatio reliance on OHM.totalSupply.

Example:

contracts/BondDepository.sol
328/**
329 * @notice calculate current bond price and remove floor if above
330 * @param _BID uint
331 * @return price_ uint
332 */
333function _bondPrice(uint256 _BID) internal returns (uint256 price_) {
334  Bond memory info = bonds[_BID];
335  price_ = info.terms.controlVariable.mul(debtRatio(_BID)).add(1000000000).div(1e7);
336  if (price_ < info.terms.minimumPrice) {
337    price_ = info.terms.minimumPrice;
338  } else if (info.terms.minimumPrice != 0) {
339    bonds[_BID].terms.minimumPrice = 0;
340  }
341}
342

343/**
344 * @notice converts bond price to DAI value
345 * @param _BID uint
346 * @return price_ uint
347 */
348function bondPriceInUSD(uint256 _BID) public view returns (uint256 price_) {
349  Bond memory bond = bonds[_BID];
350  if (address(bond.calculator) != address(0)) {
351    price_ = bondPrice(_BID).mul(bond.calculator.markdown(address(bond.principal))).div(100);
352  } else {
353    price_ = bondPrice(_BID).mul(10**IERC20Metadata(address(bond.principal)).decimals()).div(100);
354  }
355}
356

357// DEBT
358

359/**
360 * @notice calculate current ratio of debt to OHM supply
361 * @param _BID uint
362 * @return debtRatio_ uint
363 */
364function debtRatio(uint256 _BID) public view returns (uint256 debtRatio_) {
365  debtRatio_ = FixedPoint.fraction(currentDebt(_BID).mul(1e9), OHM.totalSupply()).decode112with18().div(1e18);
366}

Recommendation:

We advise the function to not adjust the minimum price as it can lead to the pricing of a bond becoming less-than-minimum under the right circumstances.

Alleviation:

The Olympus DAO team considered this exhibit but decided to retain the current behaviour of the code in place.

BDY-02M: Inexistent Validation of Terms

Description:

All arguments of the setTerms function are not sanitized and as such can be arbitrarily set to values that may be illogical or result in exploits, such as a conclusion that is beyond the expiration of the bond and other similar issues.

Example:

contracts/BondDepository.sol
97/**
98 * @notice set minimum price for new bond
99 * @param _id uint
100 * @param _controlVariable uint
101 * @param _fixedTerm bool
102 * @param _vestingTerm uint
103 * @param _expiration uint
104 * @param _conclusion uint
105 * @param _minimumPrice uint
106 * @param _maxPayout uint
107 * @param _maxDebt uint
108 * @param _initialDebt uint
109 */
110function setTerms(
111  uint256 _id,
112  uint256 _controlVariable,
113  bool _fixedTerm,
114  uint256 _vestingTerm,
115  uint256 _expiration,
116  uint256 _conclusion,
117  uint256 _minimumPrice,
118  uint256 _maxPayout,
119  uint256 _maxDebt,
120  uint256 _initialDebt
121) external onlyGuardian {
122  require(!bonds[_id].termsSet, "Already set");
123

124  Terms memory terms = Terms({controlVariable: _controlVariable, fixedTerm: _fixedTerm, vestingTerm: _vestingTerm, expiration: _expiration, conclusion: _conclusion, minimumPrice: _minimumPrice, maxPayout: _maxPayout, maxDebt: _maxDebt});
125

126  bonds[_id].terms = terms;
127  bonds[_id].totalDebt = _initialDebt;
128  bonds[_id].termsSet = true;
129}

Recommendation:

We strongly recommend some form of input sanitization to be enforced on the bond terms as in the current state the guardian has significant control over the protocol's normal operation.

Alleviation:

The Olympus DAO team considered this exhibit but decided to retain the current behaviour of the code in place.

BDY-03M: Inexplicable Optional Value of Decay

Description:

The debtDecay function relies on the vestingTerm variable of the Term struct which is meant to be an optional variable based on the documentation of the struct.

Example:

contracts/BondDepository.sol
390/**
391 * @notice amount to decay total debt by
392 * @param _BID uint
393 * @return decay_ uint
394 */
395function debtDecay(uint256 _BID) public view returns (uint256 decay_) {
396  Bond memory bond = bonds[_BID];
397  uint256 blocksSinceLast = block.number.sub(bond.lastDecay);
398  decay_ = bond.totalDebt.mul(blocksSinceLast).div(bond.terms.vestingTerm);
399  if (decay_ > bond.totalDebt) {
400    decay_ = bond.totalDebt;
401  }
402}

Recommendation:

We advise its purpose to be better defined in the struct declaration as bonds without a fixed term are possible, causing decay to misbehave.

Alleviation:

The Olympus DAO team considered this exhibit but decided to retain the current behaviour of the code in place.

Prev

sOlympusERC20.sol (OEC-S)

Next

BondTeller.sol (BTR-M)

On this page