Skip to main content
Dedaub

Pentagon Vortexes

Smart Contract Security Assessment

14.01.2021

Pentagon

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:

Category
Description
CRITICAL
Can be profitably exploited by any knowledgeable third-party attacker to drain a portion of the system’s or users’ funds OR the contract does not function as intended and severe loss of funds may result.
HIGH
Third-party attackers or faulty functionality may block the system or cause the system or users to lose funds. Important system invariants can be violated.
MEDIUM
Examples:
  • User or system funds can be lost when third-party systems misbehave.
  • DoS, under specific conditions.
  • Part of the functionality becomes unusable due to a programming error.
LOW
Examples:
  • Breaking important system invariants but without apparent consequences.
  • Buggy functionality for trusted users where a workaround exists.
  • Security issues which may manifest when the system evolves.

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


CRITICAL SEVERITY

C1

Broken functionality in the LidoHusk can result in stuck funds

C1CRITICAL

Broken functionality in the LidoHusk can result in stuck funds
open

The implementation of the reap() method of the LidoStETHHusk contract does not correctly withdraw the Husk’s sETH tokens:

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);
}


As the contract offers no other way to retrieve them, any ETH/WETH that have been deposited into Lido via the LidoHusk would be stuck.



HIGH SEVERITY

H1

Wrong amount passed into function call can cause tx failure

H1HIGH

Wrong amount passed into function call can cause tx failure
open

In Vortex::shuck an amount of gems is collected by withdrawing by as many husks as needed. To withdraw from a husk, function reap() is called:

husk.reap(want);

However it should be called as follows

husk.reap(take);

since want holds the total amount to be gathered, whereas take the amount to be withdrawn from a specific husk alone.

H2

Possible underflow may lead to loss of funds

H2HIGH

Possible underflow may lead to loss of funds
open

In Vortex::seize the ownership of a husk - along with the funds that the husk holds - is transferred to another authorized entity.

Several variables related to price per share are updated:

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;
}

However, supposing that a following transaction triggers a call on vortex::gems, the if statement that zeroes out ice is not enough to prevent a possible underflow in the unchecked calculation:

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


If the underflow check took place then this issue could cause a transaction failure, whereas now it is possible to cause the loss of all funds in the protocol.

A sketch of an attack exploiting this issue goes as follows:

Suppose that the husk to be seized holds funds equal to 'have'. And

frost > 0

awe = have + trivial

joy > 0

and trivial - frost + joy >= 0

Transaction tx1:

seize() is called.

have > gems() =>

have > awe - frost + joy =>

have > have + trivial - frost + joy =>

trivial - frost + joy < 0 : FALSE. So, ice remains > 0

Transaction tx2:

Attacker calls suck(shares), so chi() is called. Therein gems() underflows and chi() returns a huge price per share. Attacker gets all of the funds in the protocol.

We believe that it would be safer to perform:

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

Most importantly, however, we strongly advise for regular underflow/overflow checks to take place, especially when it comes to any kind of price calculations.



MEDIUM SEVERITY

M1

Possible exclusion of a husk with funds

M1MEDIUM

Possible exclusion of a husk with funds
open

In Vortex::reel an amount is withdrawn by a specific husk. If the amount to be withdrawn equals the total amount held in the husk then the husk is popped out the list of currently active husks in the vortex:

// 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();
}


However, the total amount held in the husk may have increased since the last time that the husks’ structure got updated, due to yield profits accrued in the meantime. Consequently, a husk may be popped while still holding non-zero funds in it and as a result will not be considered in the following drip()s. In essence, the extra funds held in the husk will be overseen, until it is parked again in the lot after a fresh cast.
We suggest that the current balance of the husk be queried instead, by calling Husk::probe.

M2

Wrong variable checked in require()

M2MEDIUM

Wrong variable checked in require()
open

In Vortex::fileDart the new dart value to be filed should be checked to lie within a valid range of values, however the current dart value is checked instead:

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


Consequently in case of invalid ndart value passing, not only would the dart parameter be set to an invalid value but it would also be impossible to be altered ever again.

M3

Method can fail in case of losses

M3MEDIUM

Method can fail in case of losses
open

The Vortex::shuck method is used to iterate through the lot[] array of Husks and withdraw funds if needed, to facilitate a user’s withdrawal:

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);
...
}
}

It fails to account for two kinds of failures associated with a loss:

    • The call to the reap() method of a Husk fails. One such possibility is that the husk is at a loss and has less liquidity than the cached husks[husk].have value.
    • The iteration of the lot array is complete and the required amount has not been collected. In this case the program would throw an exception to prevent an underflow of the index variable.


    The code would need to account for such cases and handle them accordingly.



LOW SEVERITY

L1

‘Seizing’ a husk does not remove its endowed status

L1LOW

‘Seizing’ a husk does not remove its endowed status
open

The implementation of the seize() method of the Vortex contract does not remove the ‘endowed’ status of the ‘seized’ husk. This can allow an authorized caller to mistakenly deposit funds to this husk (via cast()) though its funds and profits are out of reach.

L2

Inconsistent decimal value assignment

L2LOW

Inconsistent decimal value assignment
open

There are several husks which perform calculations on jewels and gems amounts using the token’s decimals information for accurate results. However, although the very same calculations are performed across some husks there some cases where dol holds the decimals of jewels token while other cases where it holds the decimals of gems token:

HarvestHusk.sol

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);
}


YearnHusk.sol

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.

A1

Unused variable

A1ADVISORY

Unused variable
open

In Vortex::drip variable held is defined and used to initialize another variable but is never used again in the function.

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


We suggest that it be omitted for clarity and slight gas optimization:

uint256 left = awe;

A2

Confusing event

A2ADVISORY

Confusing event
open

In Vortex::drip the profits accrued in the husks since the last drip are collected. For the gains of each husk an event is emitted:

emit Drip(msg.sender, gain);

This event can be quite confusing since it concerns a specific husk but no husk identification info is provided. Providing such information could be useful, for example to monitor and compare the profits and evaluate the efficiency of the husks.

We suggest that additional information be provided to such events or a single event be emitted with the total gains of the drip altogether.

A3

Variables could be immutable

A3ADVISORY

Variables could be immutable
open

Several variables could be given the immutable modifier to save gas on their uses:
AaveHusk::pool
AaveHusk::jewel
CERC20Husk::jewel
CEtherHusk::jewel
HarvestHusk::jewel
LidoStETHHusk::swap
LidoStETHHusk::jewel
StakeDAOHusk::jewel
YearnHusk::jewel

A4

Unused contract field

A4ADVISORY

Unused contract field
open

The dol field of the LidoStETHHusk contract is not used anywhere in the contract’s code, and can be removed.

A5

Compiler known issues

A5ADVISORY

Compiler known issues
info

The contracts were compiled with the Solidity compiler v0.8.10 which, at the time of writing, doesn’t have any known bugs.



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.

Dedaub