Pentagon Vortexes

Smart Contract Security Assessment

14.01.2021

SUMMARY​

ABSTRACT​

Dedaub was commissioned to perform an audit of the Pentagon Vortexes protocol at commit hash 1bb4dc5f8d2fe645bc3a541514f3ec3d28f30f10.
The primary contracts audited are listed below:

src/Vortex.sol

src/husks/HarvestHusk.sol

src/husks/CEtherHusk.sol

src/husks/StakeDAOHusk.sol

src/husks/CERC20Husk.sol

src/husks/LidoHusk.sol

src/husks/YearnHusk.sol

src/husks/AaveHusk.sol

src/Husk.sol

The audit also examined any other functionality highly related to the project, e.g. the API of the yield farming protocols with which the husk contracts interact.

SETTING & CAVEATS​

The audited codebase is of a small size at ~800LoC. The audit’s main target is security threats, i.e., what the community understanding would likely call "hacking", rather than regular use of the protocol. Functional correctness (i.e. issues in "regular use") is a secondary consideration. Functional correctness relative to low-level calculations (including units, scaling, quantities returned from external protocols) is generally most effectively done through thorough testing rather than human auditing.

ARCHITECTURE & HIGH-LEVEL RECOMMENDATIONS​

The codebase is generally well written but seems to be in a pre-release stage at this point. The project builds its own universe by introducing a new vocabulary for each entity and action taking place in the protocol. Although this comes with some initial learning curve, the vocabulary is well described and relatively easy to grasp. However no complete documentation of the overall protocol exists at the moment, which would help understanding specific purposes and design decisions. In addition, the codebase was delivered with no tests and seems to be not well tested at this point.
The protocol introduces some yield generating strategies, called “husks”, which are controlled and orchestrated by a “vortex” contract mainly regarding deposits/withdrawals of funds to external yield-farming projects. Each Vortex is related to a single investment token but may invest the user-deposited funds to a number of husks. Any user that deposits funds into a vortex gets shares of the vortex in the form of an ERC20 vortex token. The price per share is related to the amount of funds held in the vortex’s husks and the vortex contract itself, at any time. So far there is no automated mechanism taking care of the funds’ allocation across a vortex’s husks. Authorized entities are required to perform specific calls (cast, reel) on the vortex to adjust this allocation for maximum efficiency. The strategy per husk is also fairly simple at this point though the project interacts with a number of external yield farming protocols. However the team is planning to build more sophisticated strategies in the near future.

Some security considerations and recommendations are listed below.

• Centralization issues

  As is common in many new protocols, there are authorized entities accessing the contracts with considerable power over the protocol, including changing the ownership of contracts holding the user’s funds, setting fee ratios or any other configurable parameter. We list below some ideas for moderating the impact of these issues:

  • Stricter hardcoded (upper) limits of important parameters of the protocol, such as toll. Toll is currently merely checked against the highest valid value possible, i.e. 100%. Stricter limits should be considered in order to enhance and guarantee the trustworthiness of the protocol.
  • Many of the state-altering authorized operations could be protected by a timelock contract, allowing users to react to future changes.
  • Function vortex::seize may heavily hurt the trustworthiness of the protocol thus would be of the project’s benefit to be omitted. Any authorized entity can call this function to pass a husk’s control out of the Vortex, to another authorized entity. This ownership transfer happens along with all of the husk’s funds and the new owner is the only one who can access them. Consequently, the vortex’s total balance (gems) is reduced as well as its price per share (chi).

• “Caching” the funds amounts held in husks in the Vortex contract may result in faulty accounting in some cases due to stale information. See for example M1 finding below.

• Plethora of unchecked wrappers makes it hard to reason about the security of the calculations. The motivation behind the use of unchecked sections is gas savings for seemingly “safe” calculations. Throughout this audit we examined these calculations in depth, discovering one complex underflow (H2). We suggest reconsidering the unchecked sections and remove the wrapper from at least any computation that its security is not trivially reasoned upon.

• The implementation of the Vortex contract appears to be optimistic and does not always handle the possibility of a loss. Item M3 describes one such case.

• The motivation of some design decisions in the Vortexes management of Husks seems unclear:

  • The endow() and condemn() methods perform minimal state changes. For example the condemn() method does not withdraw any funds from the condemned husk.
  • The use/semantics of the lot[] array seem unclear as it could be perceived to be serving two different purposes. For example, while reel()ing from a husk it could happen that the husk gets popped out of the lot despite holding a non-zero amount of funds. This means that any profits accrued are ignored during the following drip()s (this issue is described in M1). This is problematic especially when the ignored gains are considerable and so affect the price of the vortex shares. On the other hand though, it is likely that the remaining funds are little enough so as not to be worth - as of gas costs vs. funds transferred - to be taken into consideration when shuck()ing.

VULNERABILITIES & FUNCTIONAL ISSUES​

This section details issues that affect the functionality of the contract. Dedaub generally categorizes issues according to the following severities, but may also take other considerations into account such as impact or difficulty in exploitation:

Issue resolution includes “dismissed”, by the client, or “resolved”, per the auditors.

CRITICAL SEVERITY​

function reap(uint256 want) external override chaired {
   emit Reaped(want);
   //Dedaub: exchange of ETH to ETH?
   uint256 have = swap.exchange{value: want}(0, 1, want, want - want.fmul(lossBps, 10000));
   WETH(payable(address(gem))).deposit{value: have}();
   gem.safeTransfer(msg.sender, want);
}

HIGH SEVERITY​

husk.reap(want);

husk.reap(take);

uint256 have = husks[husk].have;
// Dedaub: the following is not enough to prevent an underflow in chi()
if (have > gems()) ice = 0;

husks[husk].have = 0;
unchecked {
    awe -= have;
}

function gems() public view returns (uint256 have) {
    unchecked {
        have = awe - frost();
    }
    have += joy();
}

if ( awe - have < frost) ice = 0;

MEDIUM SEVERITY​

// Dedaub: husks[husk].have may not have been recently updated?
uint256 had = husks[husk].have;
if (had == want) {
    uint256 tpos = lot.length - 1;
    Husk otip = lot[tpos];
    lot[index] = otip;
    emit Parked(index, otip);
    emit Parked(tpos, Husk(address(0)));
    lot.pop();
}

function fileDart(uint256 ndart) external requiresAuth {
    	// Dedaub: should be 'ndart' here
    	require(dart <= 1e18, "DART_TOO_HIGH");
    	dart = ndart;
    	emit DartFiled(msg.sender, ndart);
}

function shuck(uint256 want) internal {
   uint256 need = want;
   uint256 index = lot.length - 1;
   for (; ; index--) {
      Husk husk = lot[index];
      uint256 have = husks[husk].have;
      ...
      uint256 take = have > need ? need : have;
      ...
      emit Reel(husk, take);
      husk.reap(want);
      ...
   }
}

LOW SEVERITY​

constructor(HarvestVault _jewel, address _chair) Husk(jewel.underlying(), _chair) {
    	...
    	dol = gem.decimals();
}

function reap(uint256 want) external override chaired {
    	jewel.withdraw(want.fdiv(jewel.pricePerFullShare(), dol));
}

function probe() external view override returns (uint256) {
    	return jewel.balanceOf(address(this)).fmul(jewel.pricePerFullShare(), dol);
}

constructor(YearnVault _jewel, address _chair) Husk(jewel.token(), _chair) {
      ...
    	dol = jewel.decimals();
}

function reap(uint256 want) external override chaired {
    jewel.withdraw(want.fdiv(jewel.pricePerShare(), dol), msg.sender);
}

function probe() external view override returns (uint256) {
    return jewel.balanceOf(address(this)).fmul(jewel.pricePerShare(), dol);
}

OTHER/ ADVISORY ISSUES​

This section details issues that are not thought to directly affect the functionality of the project, but we recommend considering them.

// Dedaub: unused variable 'held'
uint256 held = awe;
uint256 left = held;

uint256 left = awe;

emit Drip(msg.sender, gain);

DISCLAIMER​

The audited contracts have been analyzed using automated techniques and extensive human inspection in accordance with state-of-the-art practices as of the date of this report. The audit makes no statements or warranties on the security of the code. On its own, it cannot be considered a sufficient assessment of the correctness of the contract. While we have conducted an analysis to the best of our ability, it is our recommendation for high-value contracts to commission several independent audits, a public bug bounty program, as well as continuous security auditing and monitoring through Dedaub Security Suite.

ABOUT DEDAUB​

Dedaub offers significant security expertise combined with cutting-edge program analysis technology to secure some of the most prominent protocols in DeFi. The founders, as well as many of Dedaub's auditors, have a strong academic research background together with a real-world hacker mentality to secure code. Protocol blockchain developers hire us for our foundational analysis tools and deep expertise in program analysis, reverse engineering, DeFi exploits, cryptography and financial mathematics.