Omniscia Olympus DAO Audit

BondTeller Manual Review Findings

BondTeller Manual Review Findings

BTR-01M: Confusion of Value Denominations

TypeSeverityLocation
Logical FaultMajorBondTeller.sol:L232-L245

Description:

The percentVestedFor function assumes the bond.vested value to be a timestamp yet it represents a block.

Example:

contracts/BondTeller.sol
232/**
233 * @notice calculate how far into vesting a depositor is
234 * @param _bonder address
235 * @param _index uint
236 * @return percentVested_ uint
237 */
238function percentVestedFor(address _bonder, uint256 _index) public view returns (uint256 percentVested_) {
239    Bond memory bond = bonderInfo[_bonder][_index];
240

241    uint256 timeSince = block.timestamp.sub(bond.created);
242    uint256 term = bond.vested.sub(bond.created);
243

244    percentVested_ = timeSince.mul(1e9).div(term);
245}

Recommendation:

We strongly recommend the function to be adjusted as it should be inoperable in its current state.

Alleviation:

After discussion with the Olympus DAO team, both units are meant to represent a timestamp and as such this exhibit can be considered null.

BTR-02M: Artificial Inflation Mechanism

TypeSeverityLocation
Logical FaultMediumBondTeller.sol:L104, L110

Description:

The newBond function is meant to award bond creators with a feReward that is added to the original payout of a bond, however, this action introduces a secondary level of inflation that can also be redirected to the user themselves and could result in a significant issue if feReward is close to the minimum payout as users are incentivized to break their bonds into multiple smaller ones and set the _feo as themselves.

Example:

contracts/BondTeller.sol
86/**
87 * @notice add new bond payout to user data
88 * @param _bonder address
89 * @param _principal address
90 * @param _principalPaid uint
91 * @param _payout uint
92 * @param _expires uint
93 * @param _feo address
94 * @return index_ uint
95 */
96function newBond(
97    address _bonder,
98    address _principal,
99    uint256 _principalPaid,
100    uint256 _payout,
101    uint256 _expires,
102    address _feo
103) external onlyDepository returns (uint256 index_) {
104    treasury.mint(address(this), _payout.add(feReward));
105

106    OHM.approve(address(staking), _payout); // approve staking payout
107

108    staking.stake(_payout, address(this), true, true);
109

110    FERs[_feo] = FERs[_feo].add(feReward); // FE operator takes fee
111

112    index_ = bonderInfo[_bonder].length;
113

114    // store bond & stake payout
115    bonderInfo[_bonder].push(Bond({principal: _principal, principalPaid: _principalPaid, payout: gOHM.balanceTo(_payout), vested: _expires, created: block.timestamp, redeemed: 0}));
116}

Recommendation:

We strongly recommend this aspect of the protocol to be revised. Some potential solutions would be to retain an address of whitelisted front end operators that rewards can be re-directed to and having the feReward be based on a percentage rather than a static value.

Alleviation:

The Olympus DAO team considered this exhibit but opted not to apply a remediation for it as they consider the current mechanism sufficiently secure.

BTR-03M: Inexistent Redemption of FEO Fees

TypeSeverityLocation
Logical FaultMinorBondTeller.sol:L110

Description:

The front-end operator (FEO) fees are stored within the contract and are minted, however, there is no way to claim them by the FEOs.

Example:

contracts/BondTeller.sol
86/**
87 * @notice add new bond payout to user data
88 * @param _bonder address
89 * @param _principal address
90 * @param _principalPaid uint
91 * @param _payout uint
92 * @param _expires uint
93 * @param _feo address
94 * @return index_ uint
95 */
96function newBond(
97    address _bonder,
98    address _principal,
99    uint256 _principalPaid,
100    uint256 _payout,
101    uint256 _expires,
102    address _feo
103) external onlyDepository returns (uint256 index_) {
104    treasury.mint(address(this), _payout.add(feReward));
105

106    OHM.approve(address(staking), _payout); // approve staking payout
107

108    staking.stake(_payout, address(this), true, true);
109

110    FERs[_feo] = FERs[_feo].add(feReward); // FE operator takes fee
111

112    index_ = bonderInfo[_bonder].length;
113

114    // store bond & stake payout
115    bonderInfo[_bonder].push(Bond({principal: _principal, principalPaid: _principalPaid, payout: gOHM.balanceTo(_payout), vested: _expires, created: block.timestamp, redeemed: 0}));
116}

Recommendation:

We advise some form of pull-pattern to be applied where FEOs are able to invoke a function to retrieve all fees they have accumulated.

Alleviation:

A fee redemption mechanism was introduced in the codebase in the form of the getReward function that transfers the necessary OHM outward to the user.

BTR-04M: Inexistent Validation of Non-Zero Redemption

TypeSeverityLocation
Input SanitizationMinorBondTeller.sol:L148

Description:

The redeem function should validate that a non-zero redemption is being performed.

Example:

contracts/BondTeller.sol
130/**
131 *  @notice redeem bond for user
132 *  @param _bonder address
133 *  @param _indexes calldata uint[]
134 *  @return uint
135 */
136function redeem(address _bonder, uint256[] memory _indexes) public returns (uint256) {
137    uint256 dues;
138    for (uint256 i = 0; i < _indexes.length; i++) {
139        Bond memory info = bonderInfo[_bonder][_indexes[i]];
140

141        if (pendingFor(_bonder, _indexes[i]) != 0) {
142            bonderInfo[_bonder][_indexes[i]].redeemed = block.timestamp; // mark as redeemed
143

144            dues = dues.add(info.payout);
145        }
146    }
147

148    dues = gOHM.balanceFrom(dues);
149

150    emit Redeemed(_bonder, dues);
151    pay(_bonder, dues);
152    return dues;
153}

Recommendation:

We advise this to be done so by ensuring that dues is non-zero beyond conversion.

Alleviation:

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

Prev

BondDepository.sol (BDY-M)

Next

ERC20.sol (ERC-M)

On this page