Auditor Quick Start

The information provided by Aloe Labs, Inc. (“we,” “us” or “our”) on docs.aloe.capital (the “Site”) is for general informational purposes only. All information on the Site is provided in good faith, however we make no representation or warranty of any kind, express or implied, regarding the accuracy, adequacy, validity, reliability, availability or completeness of any information on the Site.
Under no circumstance shall we have any liability to you for any loss or damage of any kind incurred as a result of the use of the site or reliance on any information provided on the site. Your use of the site and your reliance on any information on the site is solely at your own risk.

Potential Problem Areas

This page outlines areas of the protocol that (in our opinion) deserve extra scrutiny. These are known complexities, which means we've thought through them, and believe them to be bug-free. You are more likely to find bugs related to unknown complexity, i.e. things that we haven't thought through.

Still, we think these are useful starting points to get your head around the protocol. If you understand what's going on in these examples, you'll be well-positioned to explore the rest of the protocol.

Ill-considered interactions between Lender, Borrower, and Factory, especially inside external calls.

This would include explicit callbacks, such as Lender's flash loans and Borrower's modify, and also any calls to user-specified addresses like Borrower's rescue.

State changes in Lender that impact Borrower, and vice versa

After a Borrower has been warned, but before it's liquidated, there's an opportunity to make it healthy and avoid liquidation. If this is done via a call to Borrower.modify, the warning will be cleared, and further interactions are normal. However, if it's done by a 3rd-party via Lender.repay (with the borrower set as the beneficiary), the warning will not be cleared.

In order to clear the warning, the borrower's owner must call Borrower.modify, even if the callback is a no-op. If the account goes unhealthy again before they've done this, liquidations can proceed without an additional warning, because the old unleashLiquidationTime is still set.

We noticed this behavior during the beta. We are not considering it a bug in and of itself, so long as we make users aware of it. However, if there are related/similar issues that we've missed, we'd love to know about them.

Overflow in Lender

The basic Lender functionality (tracking deposits, borrows, and interest) is quite straightforward, and we're fairly certain it's sufficiently precise (see napkin.py).

However, there are two features that make things more complicated:

Built-in referral program
Built-in rewards

The referral program (which you can find by searching the code for "courier") requires us to track the user's principle in addition to their current shares. Both values are uint112s, and we pack them into the same balances mapping in Lender. We frequently use unchecked math. Some questions you might ask: Can one value overflow into the other? What happens if convertToAssets(shares) is less than the user's stored principle? Would subtraction then result in underflow? What else would that scenario imply regarding bad-debt or calculation precision?

The rewards logic is intentionally approximate, i.e. we do not care about 18 decimal precision. However, it should be somewhat accurate, and if it's wrong, it should always underestimate rewards rather than pay out too much. Also, if something goes wrong with rewards, we don't want it to brick the contract or impair users from withdrawing. These two qualities should hold for any configuration (namely, any rewards rate).

Price precision in Borrower

One of the key protocol invariants is that every borrower needs to be healthy after modification. The health check is defined in BalanceSheet.sol. Note that the calculations are done in terms of token1－will this create problems when token1 has few decimals (think USDC)? Have we rounded in the appropriate direction where applicable?

The same sort of questions can be asked at various points in Borrower.liquidate. We don't want to pay out too large an incentive to liquidators, so rounding and price precision are key.

Oracle griefing

Is there any way to delay or prevent updates on the VolatilityOracle? Are users guaranteed to be able to call Factory.pause when the oracle seems to be manipulated?

Issues with in-memory state tracking

Ways that governance might brick or freeze the contract (assume governance is an attacker)

Obviously governance may set sub-optimal parameters. That's not a bug. What would be a bug is if governance could circumvent the min/max constraints we've set, or if it could otherwise brick the contracts and prevent people from withdrawing.

One way we've defended against this is by having calls to the RateModel fail safely. Is there anywhere else we should consider that kind of logic?

Special-cases related to the RESERVE address and couriers

We believe the RESERVE address can operate without restriction, i.e. it can call any function in the protocol without causing accounting errors. Where it needs to be limited, we believe it is. For example, Lender.deposit prevents it from having a courier. But are we missing anything? Many of our invariants in LenderHarness have special logic to account for the RESERVE address, and while we think everything is correct, we'd like to have more eyes on it.

Similarly, couriers are free to call any function, with one exception: they cannot claim rewards. Is there anything else we should restrict? Are ERC20 events emitted properly for them?

Ways in which Rewards could be manipulated?

Things that happen if markets are created for fake/nefarious Uniswap pools

Aloe II does not support non-standard ERC20 tokens or fake Uniswap pools. This is not a bug. However, if creating a market for such tokens/pools could interfere with the operation of legitimate pools, that is a bug. In general, any way in which one market could influence another market is problematic.

One area where this might show up is in Factory.claimRewards because it's calling functions across multiple markets at the same time. Should we be concerned about reentrancy or other attacks here?

Masking of Borrower.slot0

To maximize gas efficiency, we use bit masking to read/write 3 different values in a single storage slot of Borrower. These 3 values are:

positions (208 bits): The lower and upper bounds of up to 3 Uniswap positions, encoded using Positions.zip. Since that only takes 144 bits (24 * 6), there are 64 bits leftover. In theory the user could use those extra bits to store any information they want, without increasing gas costs.
unleashLiquidationTime (40 bits): The time at which swap-style liquidations can take place. Set in warn, erased in liquidate and modify.
state (7 bits): An enum indicating whether the Borrower is in a clean state (Ready), locked to reentrancy (Locked), or in the modify callback (InModifyCallback).
dirt (1 bit): The highest bit of slot0 is set to 1 so that we're guaranteed to be replacing a non-zero value with another non-zero value, rather than overwriting with 0. It's rare that this would matter, since other bits are usually non-zero anyway, but sometimes it saves gas.

It's absolutely critical that these values do not bleed into one another. It's also critical that positions does not change outside of modify, and when it does change, we must verify that there are no duplicate positions (otherwise someone's Borrower may appear healthy when it's not). The one exception to this is if there are no liabilities－in that case, we don't care if there are duplicates, because the account is obviously healthy regardless.

The duplicate check is performed in Positions.extract. That function should only care about the first 144 bits, which is why we can pass slot0 into it directly without first masking it.

Last updated 1 year ago