Shell Protocol

1 Executive Summary

This report presents the results of our engagement with Counterparty to review Shell Protocol.

The review was conducted over the course of two weeks and two days, from June 22 to July 7 2020 by Daniel Luca and Gonçalo Sá. A total of 22 person-days were spent. This review is following another previous 1-day review we provided for the client.

During the first week, we started to learn how the system works by having a few calls with the client and by reading the provided documents and the available source code. We set up a meeting with the development team on Monday to explain our process, understand the system, agree on the overall scope and purpose of the audit, and ask for a commit hash.

We had some initial problems with compiling the code because we were not familiar with the framework, and not all of the changes were pushed to the repository. Over the next few days, we went back and forth, trying to identify the problems and coming up with solutions on how to make the code compilable. Once we agreed on a pretty good version, we locked in the commit hash 2f0f9d6c5a6ba471ae88f14da1f1b3e8470332b0. A complete list of files can be found in the Appendix.

On Tuesday, we had another call with the development team to discuss the high-level overview of the system, roughly getting into the math behind the balancing mechanics. We also asked for a walk-through of the system, to understand how a user is supposed to interact with it. We got familiar with the core functionality of the system, namely, how the balancing is done and how tokens flow within the system. We also discussed why some decisions were made as they are, specifically how internal accounting is done, how external calls are done, and why they exist, how the assimilators work in principle.

On Wednesday, we continued to do our manual review and set up a new meeting with the client to discuss initial findings, ask questions, and continue the code walk-through. Our main focus was to follow the lifetime of a simple token swap transaction throughout the codebase.

Over the following days, we directed our efforts towards understanding the system, trying to find issues and edge cases. A number of issues were filed to be included in the report.

It was clear to us that it is vital to help the development team change the way they are currently developing the application. A number of systemic problems were identified and explained in detail below, in the Action Items section.

On Friday, we set up another meeting with the client where we discussed that our focus would be split into finding more security issues with the system, but also address the overall methodology of the development process.

In the second week we continued to file issues while categorizing them into groups relating to complexity, fragility, and testing. Each of the groups is referred to in the Action Items. Other security issues not relating to the aforementioned groups were filed in the review.

We continued to have daily sync meetings to discuss any issues we might have found and use our group knowledge to validate and test different attack vectors. Most of the time spent in the second week was to file new issues, validate attack vectors, and put the report together in a format that the audit team believes to deliver the most value to the development team.

At the end of the second week, we had a meeting with the development team where we discussed a few other attack vectors, power centralization issues, and other parts of the code that were not completely clear.

1.1 Mitigations Review Update

The Shell Protocol team diligently addressed all of the issues present in the report. This effort entailed huge code transformations that were completed at a fast pace.

The biggest transformation was the creation of a “pool partitioning” mechanism to tackle the lock-up conditions stemming from the pool balancing loop needed for the correct functioning of the system.

Since the beginning of the audit, there were clear improvements in both the quality of the code style and the attention to the product’s security.

The auditing team would also like to make notice of the fact that the codebase was in a developing state by the time of the audit and therefore strong changes were sure to ensue.

2 Scope

Our review focused on the commit hash 2f0f9d6c5a6ba471ae88f14da1f1b3e8470332b0. The list of files in scope can be found in the Appendix.

2.1 Objectives

Together with the Counterparty team, we identified the following priorities for our review:

  1. Ensure that the system is implemented consistently with the intended functionality and without unintended edge cases.
  2. Identify known vulnerabilities particular to smart contract systems, as outlined in our Smart Contract Best Practices, and the Smart Contract Weakness Classification Registry.
  3. Try to find ways to reduce gas costs.

3 System Overview

Shell Protocol is a conjunction of a liquidity pool and AMM for stablecoins that is designed to have no slippage beyond the liquidity fee and to pass arbitrage profits on to the liquidity providers (LPs, from now on). To achieve this goal, Shell Protocol implements a bonding surface in its core logic made up of several smaller, locally-defined bonding surfaces.

Even though Shell Protocol is made of clearly delineated business logic modules, the codebase under review implements them in a way heavily intertwined way. As such, it is easier to distinguish and categorize these components based on their logical functions rather than specific files or contracts.

Loihi is the name of this version of Shell Protocol’s codebase and the one we will be describing in the next few paragraphs.

The two main logical components of the system are shell s and assimilators. With a shell being the most central part of the system and the assimilators being the middleware connecting the different financial instruments to the core liquidity and swap logic for each pool.

At the time of the audit, a formalism of the bonding surface implemented by Loihi was provided to the audit team and can be found here.

However, since then, an updated version of the formalism was formulated and can be found in the whitepaper.

3.1 Shell

A shell is, as stated, the core, logical part of Shell Protocol. Each instance of a pool will have exactly one shell.

The shell is the data structure present within the smart contract system that ties all the other components together. Encapsulated inside the resulting compiled smart contract that makes up a shell there is logic for:

  • the core mathematical components for the loss function creating the bonding curve (Shells.sol, Loihi.sol)
  • ERC20 implementation of the shell token (ShellsExternal.sol, Shells.sol)
  • depositing liquidity in the pool (Loihi.sol)
  • withdrawing liquidity from the pool (Loihi.sol)
  • swapping two tokens supported by the pool (Loihi.sol)
  • administrative functions that rule the pool’s parameters (Controller.sol, Loihi.sol, LoihiRoot.sol)

Most of the files mentioned above take the form of all-internal-method libraries that get fully appended to Loihi.sol ’s and LoihiRoot.sol ’s bytecode, since they are the only contracts in the set.

In addition to the components, Shell Protocol is using safe math libraries to handle both 256-bit unsigned integer and 128-bit 64.64 fixed-point decimal arithmetics with no over/underflow.

All the mathematical components handling the fixed fee, halt enforcement parameters and slippage calculations are using 64.64 fixed-point decimals, internally, using Solidity’s elementary 128-bit signed integer type.

All the math-related methods are contained within Shells.sol with some amount of preparation for the calculations being done inside Loihi.sol, as well.

In Loihi.sol, the system prepares the data that is fed into the implementation of the bonding surface in Shells.sol. Loihi queries the token balances for the set of supported stablecoins and their derivatives, creates the necessary derived variables needed for the calculations (mostly sums of all the token balances, through the methods getLiquidityData and getSwapData) and then calls the relevant methods for the calculations (calculateLiquidityMembrane and calculateTrade, related to the prior methods in the respective order) from Shells.sol.

The Loihi contract is then responsible for propagating the changes (and effectively writing them to storage) to the omega parameter of the pool and calling the respective assimilator methods to credit or withdraw the relevant amount of tokens while implementing boundary checks for these values at the same time.

Administrative Functions

Most of the administrative functions logic is implemented in Controller.sol with some of it being implemented in Loihi.sol (more specifically the ability to remove an assimilator and the ability to send tokens from the contract to an address of their choice).

The functions present in the Controller allow the administrator to set new parameters for the pool, include a new supported asset (stablecoin) and include a new assimilator for any supported asset.

3.2 Assimilators

The assimilators are the middleware between a shell and the different DeFi systems it needs to interact in its set of supported assets and their derivatives.

They act, in essence, as a delegatecall proxy system to all the different stablecoins and their derivatives in order for the pool to be able to interactively balance itself and allow LPs to provide liquidity.

In the current architecture, assimilators necessarily need to take the shape of proxies to externally deployed contracts. This is due to the fact that each of the supported assets and its derivatives have differently named methods and even control flows that need to be followed in order to interact with their token implementations properly.

Not only are assimilators an abstraction to the different interfaces and accounting models of each of the supported assets and derivatives but a necessary instrument in the normalization of each of these external tokens’ value internally to Shell Protocol.

In the file Assimilators.sol, all the methods present are merely internal functions meant to delegate execution to the each relevant implementation (given the relevant token) at runtime. The critical part of the assimilator architecture is present in the assimilators/ directory inside the repository under review.

The actual implementations of the assimilators (only meant to be delegatecalled) mostly implement the same interface and are the components responsible for both interfacing with the external DeFi systems and also make the correctness checks about the success of these necessary sub-calls. Assimilators are also keeping the consistency by typecasting between the conventional unsigned integer used for balances in ERC20-compatible tokens and the 64.64 fixed-point decimal used by the shell, internally.

4 Action Items

4.1 Reduce overall complexity

Mitigations Review Update

Comment from the development team:

Previously, we were utilizing libraries with the “using library for type” convention. This made the code difficult to understand.

Now our library use is well named and with the exception of mathematical operations is employed using the normal call syntax like “Library.function(argument, argument)”. Combined with descriptive names for the libraries, it is easy to see where the code is flowing to.

Although we now make use of a total of 9 non-math libraries (including internal and external libraries), they are well named and easy to reason with.

Complexity comes at the cost of security. Complex systems are harder to understand, harder to test, and harder to maintain.

For smart contract systems, the fault-intolerant environment of the EVM necessarily demands that security is the highest priority. Therefore, it should be a design goal of all smart contract systems to reduce complexity and make logic explicit wherever possible.

The Shell Protocol is a highly complex system:

  • There is a deep library usage that spans between multiple files, even having libraries include other libraries a few levels deep.
  • The mathematical model is not completely and clearly defined; the document explaining the math powering the system has not reached a final version.
  • A large number of assimilators can be included as part of the system. Each of them has to be reviewed in the context of the system, but also in the context of the token being supported because adding an assimilator which is incorrectly implemented can put the system at risk.
  • Fixed point math operations are used in the system, but the libraries were changed, and some of the implementations are duplicated in multiple contracts.
  • A common theme throughout the system is to use delegatecall, which creates a huge trust issue since the owner can, at any point in time, add an assimilator that steals all the tokens in the system.
  • There are inconsistencies in function names and variable names; these should all be addressed. For example “Assimilators” used to be called “Adapters”, and some of the function names still refer to “Adapters”, we have includeAssimilator and excludeAdapter.

Recommendation:

Reducing overall complexity is no simple task, and addressing this system’s complexity will require careful thought and consideration outside of the scope of this review. In general, prioritize the following concepts:

  • Optimize for readability. Ensure that code is as easy to understand as possible. Implement clear and consistent naming conventions, group similar functions within the same file, and generally attempt to structure and organize the code so that humans can read and understand it best.
  • Remove commented-out code. Remove old code that was used for tests or for setting up local environments and find other ways to mock or configure the system without the need to change code.
Issues Priority
Remove commented out code from the repository High
Remove debugging code from the repository Medium
Use consistent interfaces for functions in the same group Medium
Use one file for each contract or library Low

4.2 Increase the overall quality and quantity of testing

Mitigations Review Update

Comment from the development team:

The failing tests existed because we made minute changes to our present model (changes in applying the base fee - “epsilon”), so in a sense, rather than failing they just need updating. Many of them are also an artifact of architecting the tests in such a way that they can be run against arbitrary parameter sets - or in different “suites”.

Several findings of this assessment suggest that Shell Protocol is inadequately tested:

  • Almost half of the tests fail.
  • There is no continuous integration system.
  • There is no report about code coverage. We do not know if the tests cover the whole codebase. This makes it likely that not all functionality is well tested.
  • Some of the changes implemented in the fork libraries do not implement the intended functionality.

Recommendation:

Implementing a robust, complete test suite requires careful consideration outside of the scope of this review. In general, prioritize the following concepts:

  • Write tests that encapsulate the specification. Tests should address each of a system’s requirements. A system’s requirements should be clearly defined within the system design specification.
  • Try to develop new functionality by writing tests first. Test-driven development makes sure that all of the written code is accompanied by a test.
  • Implement a continuous integration system. Using one of the platforms that offer CI/CD services and implements a list of actions that do compilation, tests, code coverage, and panics when the smallest piece does not check out.
Issues Priority
Tests should not fail High
Code coverage should be close to 100% Medium

4.3 Address codebase fragility

Mitigations Review Update

Comment from the development team:

Software is considered “fragile” when issues or changes in one part of the system can have side-effects in conceptually unrelated parts of the codebase. Fragile software tends to break easily and may be challenging to maintain.

Our assessment uncovered that for each swap in the system, all of the enabled assets run code. That means that if one of the enabled tokens blacklists the exchange, all of the tokens are locked in the system unless a backdoor exists.

Recommendation:

Building an anti-fragile system requires careful thought and consideration outside of the scope of this review. In general, prioritize the following concepts:

  • Follow checks-effects-interactions pattern. The checks-effects-interactions should be implemented throughout the code. Also, functions’ return values should always be checked for correctness.
  • Follow the single-responsibility principle of functions. This principle states that functions should have responsibility for a single part of the system’s functionality and that their purpose should be narrowly-aligned with that responsibility. Avoid functions that “do everything” (like setGovernanceParameter), and avoid functions that touch every other function (like funding and markPrice).
Issues Priority
Functions do not check the correctness of the arguments High
Math library’s fork has problematic changes Medium
Check return values for both internal and external calls Medium

5 Security Specification

This section describes, from a security perspective, the expected behavior of the system under audit. It is not a substitute for documentation. The purpose of this section is to identify specific security properties that were validated by the audit team.

5.1 Actors

The relevant actors are listed below with their respective abilities:

  • Non-privileged access actors
    • Pool user (i.e., non-privileged user with no shell tokens in their possession)
      • Can swap assets supported by the pool.
    • Liquidity provider
      • All of the above.
      • Can deposit supported assets into the pool.
      • Can withdraw its share of supported assets from the pool (relative to the amount of shell tokens they hold).
  • Privileged access actors
    • Administrator
      • All of the above.
      • Setting all the parameters of the pool at anytime.
      • Adding supported assets.
      • Adding supported assimilators (basically setting an address to which execution is delegated, no restrictions).

5.2 Important Security Properties

The following is a non-exhaustive list of security properties that were verified in this audit:

  • Non-privileged access actors
    • Pool user
      • Cannot swap assets that are unsupported by the pool.
      • Cannot swap an asset bypassing the fee calculation.
      • Cannot bypass the depositing of the intake token.
    • Liquidity provider
      • Cannot bypass the fee calculation when depositing or withdrawing assets.
      • Cannot mint or burn tokens in a proportion not relative to their held shell tokens.
        • By repeated action, cannot drain the pool by exploiting a bad implementation of the pre-fee-calculation parameters.

Please note that some other properties were reviewed in addition to these ones. The ones that were proven to be untrue are instead represented as issues in this same report.

6 Issues

Each issue has an assigned severity:

  • Minor issues are subjective in nature. They are typically suggestions around best practices or readability. Code maintainers should use their own judgment as to whether to address such issues.
  • Medium issues are objective in nature but are not security vulnerabilities. These should be addressed unless there is a clear reason not to.
  • Major issues are security vulnerabilities that may not be directly exploitable or may require certain conditions in order to be exploited. All major issues should be addressed.
  • Critical issues are directly exploitable security vulnerabilities that need to be fixed.

6.1 Unexpected response in an assimilator’s external call can lock-up the whole system Major ✓ Fixed

Resolution

Comment from the development team:

When this was brought to our attention, it made the most sense to look at it from a bird’s eye view. In the event that an assimilator does seize up either due to smart contract malfunctioning or to some type of governance decision in one of our dependencies, then depending on the severity of the event, it could either make it so that that particular dependency is unable to be transacted with or it could brick the pool altogether.

In the case of the latter severity where the pool is bricked altogether for an extended period of time, then this means the end of that particular pool’s life. In this case, we find it prudent to allow for the withdrawal of any asset still functional from the pool. Should such an event transpire, we have instituted functionality to allow users to withdraw individually from the pool’s assets according to their Shell balances without being exposed to the inertia of the incapacitated assets.

In such an event, the owner of the pool can now trigger a partitioned state which is an end of life state for the pool in which users send Shells as normal until they decide to redeem any portion of them, after which they will only be able to redeem the portion of individual asset balances their Shell balance held claims on.

Description

The assimilators, being the “middleware” between a shell and all the external DeFi systems it interacts with, perform several external calls within their methods, as would be expected.

An example of such a contract is mainnetSUsdToASUsdAssimilator.sol (the contract can be found here).

The problem outlined in the title arises from the fact that Solidity automatically checks for the successful execution of the underlying message call (i.e., it bubbles up assertions and reverts) and, therefore, if any of these external systems changes in unexpected ways the call to the shell will revert itself.

This problem is immensely magnified by the fact that all the external methods in Loihi dealing with deposits, withdraws, and swaps rebalance the pool and, as a consequence, all of the assimilators for the reserve tokens get called at some point.

In summary, if any of the reserve tokens start, for some reason, refusing to complete a call to some of their methods, the whole protocol stops working, and the tokens are locked in forever (this is assuming the development team removes the safeApprove function from Loihi, v. https://github.com/ConsenSys/shell-protocol-audit-2020-06/issues/10).

Recommendation

There is no easy solution to this problem since calls to these external systems cannot simply be ignored. Shell needs successful responses from the reserve assimilators to be able to function properly.

One possible mitigation is to create a trustless mechanism based on repeated misbehavior by an external system to be able to remove a reserve asset from the pool.

Such a design could consist of an external function accessible to all actors that needs X confirmations over a period of Y blocks (or days, for that matter) with even spacing between them to be able to remove a reserve asset.

This means that no trust to the owners is implied (since this would require the extreme power to take user’s tokens) and still maintains the healthy option of being able to remove faulty tokens from the pool.

6.2 Certain functions lack input validation routines Major ✓ Fixed

Resolution

Comment from the development team:

  1. Now all functions in the Orchestrator revert on incorrect arguments.
  2. All functions in Loihi in general revert on incorrect arguments.

Description

The functions should first check if the passed arguments are valid first. The checks-effects-interactions pattern should be implemented throughout the code.

These checks should include, but not be limited to:

  • uint should be larger than 0 when 0 is considered invalid
  • uint should be within constraints
  • int should be positive in some cases
  • length of arrays should match if more arrays are sent as arguments
  • addresses should not be 0x0

Examples

The function includeAsset does not do any checks before changing the contract state.

src/Loihi.sol:L59-L61

function includeAsset (address _numeraire, address _nAssim, address _reserve, address _rAssim, uint256 _weight) public onlyOwner {
    shell.includeAsset(_numeraire, _nAssim, _reserve, _rAssim, _weight);
}

The internal function called by the public method includeAsset again doesn’t check any of the data.

src/Controller.sol:L77-L97

function includeAsset (Shells.Shell storage shell, address _numeraire, address _numeraireAssim, address _reserve, address _reserveAssim, uint256 _weight) internal {

    Assimilators.Assimilator storage _numeraireAssimilator = shell.assimilators[_numeraire];

    _numeraireAssimilator.addr = _numeraireAssim;

    _numeraireAssimilator.ix = uint8(shell.numeraires.length);

    shell.numeraires.push(_numeraireAssimilator);

    Assimilators.Assimilator storage _reserveAssimilator = shell.assimilators[_reserve];

    _reserveAssimilator.addr = _reserveAssim;

    _reserveAssimilator.ix = uint8(shell.reserves.length);

    shell.reserves.push(_reserveAssimilator);

    shell.weights.push(_weight.divu(1e18).add(uint256(1).divu(1e18)));

}

Similar with includeAssimilator.

src/Loihi.sol:L63-L65

function includeAssimilator (address _numeraire, address _derivative, address _assimilator) public onlyOwner {
    shell.includeAssimilator(_numeraire, _derivative, _assimilator);
}

Again no checks are done in any function.

src/Controller.sol:L99-L106

function includeAssimilator (Shells.Shell storage shell, address _numeraire, address _derivative, address _assimilator) internal {

    Assimilators.Assimilator storage _numeraireAssim = shell.assimilators[_numeraire];

    shell.assimilators[_derivative] = Assimilators.Assimilator(_assimilator, _numeraireAssim.ix);
    // shell.assimilators[_derivative] = Assimilators.Assimilator(_assimilator, _numeraireAssim.ix, 0, 0);

}

Not only does the administrator functions not have any checks, but also user facing functions do not check the arguments.

For example swapByOrigin does not check any of the arguments if you consider it calls MainnetDaiToDaiAssimilator.

src/Loihi.sol:L85-L89

function swapByOrigin (address _o, address _t, uint256 _oAmt, uint256 _mTAmt, uint256 _dline) public notFrozen returns (uint256 tAmt_) {

    return transferByOrigin(_o, _t, _dline, _mTAmt, _oAmt, msg.sender);

}

It calls transferByOrigin and we simplify this example and consider we have _o.ix == _t.ix

src/Loihi.sol:L181-L187

function transferByOrigin (address _origin, address _target, uint256 _dline, uint256 _mTAmt, uint256 _oAmt, address _rcpnt) public notFrozen nonReentrant returns (uint256 tAmt_) {

    Assimilators.Assimilator memory _o = shell.assimilators[_origin];
    Assimilators.Assimilator memory _t = shell.assimilators[_target];

    // TODO: how to include min target amount
    if (_o.ix == _t.ix) return _t.addr.outputNumeraire(_rcpnt, _o.addr.intakeRaw(_oAmt));

In which case it can call 2 functions on an assimilatior such as MainnetDaiToDaiAssimilator.

The first called function is intakeRaw.

src/assimilators/mainnet/daiReserves/mainnetDaiToDaiAssimilator.sol:L42-L49

// transfers raw amonut of dai in, wraps it in cDai, returns numeraire amount
function intakeRaw (uint256 _amount) public returns (int128 amount_, int128 balance_) {

    dai.transferFrom(msg.sender, address(this), _amount);

    amount_ = _amount.divu(1e18);

}

And its result is used in outputNumeraire that again does not have any checks.

src/assimilators/mainnet/daiReserves/mainnetDaiToDaiAssimilator.sol:L83-L92

// takes numeraire amount of dai, unwraps corresponding amount of cDai, transfers that out, returns numeraire amount
function outputNumeraire (address _dst, int128 _amount) public returns (uint256 amount_) {

    amount_ = _amount.mulu(1e18);

    dai.transfer(_dst, amount_);

    return amount_;

}

Recommendation

Implement the checks-effects-interactions as a pattern to write code. Add tests that check if all of the arguments have been validated.

Consider checking arguments as an important part of writing code and developing the system.

6.3 Remove Loihi methods that can be used as backdoors by the administrator Major ✓ Fixed

Resolution

Issue was partly addressed by the development team. However, the feature to add new assimilators is still present and that ultimately means that the administrators have power to run arbitrary bytecode.

Updated remediation response Since the development team still hadn’t fully settled on a strategy for a mainnet launch, this was left as a residue even after the audit mitigation phase. However, at launch time, this issue was no longer present and all the assimilators are now defined at deploy-time, it is fully resolved.

Description

There are several functions in Loihi that give extreme powers to the shell administrator. The most dangerous set of those is the ones granting the capability to add assimilators.

Since assimilators are essentially a proxy architecture to delegate code to several different implementations of the same interface, the administrator could, intentionally or unintentionally, deploy malicious or faulty code in the implementation of an assimilator. This means that the administrator is essentially totally trusted to not run code that, for example, drains the whole pool or locks up the users’ and LPs’ tokens.

In addition to these, the function safeApprove allows the administrator to move any of the tokens the contract holds to any address regardless of the balances any of the users have.

This can also be used by the owner as a backdoor to completely drain the contract.

src/Loihi.sol:L643-L649

function safeApprove(address _token, address _spender, uint256 _value) public onlyOwner {

    (bool success, bytes memory returndata) = _token.call(abi.encodeWithSignature("approve(address,uint256)", _spender, _value));

    require(success, "SafeERC20: low-level call failed");

}

Recommendation

Remove the safeApprove function and, instead, use a trustless escape-hatch mechanism like the one suggested in issue 6.1.

For the assimilator addition functions, our recommendation is that they are made completely internal, only callable in the constructor, at deploy time.

Even though this is not a big structural change (in fact, it reduces the attack surface), it is, indeed, a feature loss. However, this is the only way to make each shell a time-invariant system.

This would not only increase Shell’s security but also would greatly improve the trust the users have in the protocol since, after deployment, the code is now static and auditable.

6.4 Assimilators should implement an interface Major ✓ Fixed

Resolution

Comment from the development team:

They now implement the interface in “src/interfaces/IAssimilator.sol”.

Description

The Assimilators are one of the core components within the application. They are used to move the tokens and can be thought of as a “middleware” between the Shell Protocol application and any other supported tokens.

The methods attached to the assimilators are called throughout the application and they are a critical component of the whole system. Because of this fact, it is extremely important that they behave correctly.

A suggestion to restrict the possibility of errors when implementing them and when using them is to make all of the assimilators implement a unique specific interface. This way, any deviation would be immediately observed, right when the compilation happens.

Examples

Consider this example. The user calls swapByOrigin.

src/Loihi.sol:L85-L89

function swapByOrigin (address _o, address _t, uint256 _oAmt, uint256 _mTAmt, uint256 _dline) public notFrozen returns (uint256 tAmt_) {

    return transferByOrigin(_o, _t, _dline, _mTAmt, _oAmt, msg.sender);

}

Which calls transferByOrigin. In transferByOrigin, if the origin index matches the target index, a different execution branch is activated.

src/Loihi.sol:L187

if (_o.ix == _t.ix) return _t.addr.outputNumeraire(_rcpnt, _o.addr.intakeRaw(_oAmt));

In this case we need the output of _o.addr.intakeRaw(_oAmt).

If we pick a random assimilator and check the implementation, we see the function intakeRaw needs to return the transferred amount.

src/assimilators/mainnet/daiReserves/mainnetCDaiToDaiAssimilator.sol:L52-L67

// takes raw cdai amount, transfers it in, calculates corresponding numeraire amount and returns it
function intakeRaw (uint256 _amount) public returns (int128 amount_) {

    bool success = cdai.transferFrom(msg.sender, address(this), _amount);

    if (!success) revert("CDai/transferFrom-failed");

    uint256 _rate = cdai.exchangeRateStored();

    _amount = ( _amount * _rate ) / 1e18;

    cdai.redeemUnderlying(_amount);

    amount_ = _amount.divu(1e18);

}

However, with other implementations, the returns do not match. In the case of MainnetDaiToDaiAssimilator, it returns 2 values, which will make the Loihi contract work in this case but can misbehave in other cases, or even fail.

src/assimilators/mainnet/daiReserves/mainnetDaiToDaiAssimilator.sol:L42-L49

// transfers raw amonut of dai in, wraps it in cDai, returns numeraire amount
function intakeRaw (uint256 _amount) public returns (int128 amount_, int128 balance_) {

    dai.transferFrom(msg.sender, address(this), _amount);

    amount_ = _amount.divu(1e18);

}

Making all the assimilators implement one unique interface will enforce the functions to look the same from the outside.

Recommendation

Create a unique interface for the assimilators and make all the contracts implement that interface.

6.5 Assimilators do not conform to the ERC20 specification Medium ✓ Fixed

Resolution

Comment from the development team:

All calls to compliant ERC20s now check for return booleans. Non compliant ERC20s are called with a function that checks for the success of the call.

Description

The assimilators in the codebase make heavy usage of both the transfer and transferFrom methods in the ERC20 standard.

Quoting the relevant parts of the specification of the standard:

Transfers _value amount of tokens to address _to, and MUST fire the Transfer event. The function SHOULD throw if the message caller’s account balance does not have enough tokens to spend.

The transferFrom method is used for a withdraw workflow, allowing contracts to transfer tokens on your behalf. This can be used for example to allow a contract to transfer tokens on your behalf and/or to charge fees in sub-currencies. The function SHOULD throw unless the _from account has deliberately authorized the sender of the message via some mechanism.

We can see that, even though it is suggested that ERC20-compliant tokens do throw on the lack of authorization from the sender or lack of funds to complete the transfer, the standard does not enforce it.

This means that, in order to make the system both more resilient and future-proof, code in each implementation of current and future assimilators should check for the return value of both transfer and transferFrom call instead of just relying on the external contract to revert execution.

The extent of this issue is only mitigated by the fact that new assets are only added by the shell administrator and could, therefore, be audited prior to their addition.

Non-exhaustive Examples

src/assimilators/mainnet/daiReserves/mainnetDaiToDaiAssimilator.sol:L45

dai.transferFrom(msg.sender, address(this), _amount);

src/assimilators/mainnet/daiReserves/mainnetDaiToDaiAssimilator.sol:L64

dai.transfer(_dst, _amount);

Recommendation

Add a check for the return boolean of the function.

Example:

require(dai.transferFrom(msg.sender, address(this), _amount) == true);

6.6 Access to assimilators does not check for existence and allows delegation to the zeroth address Medium ✓ Fixed

Resolution

Comment from the development team:

All retrieval of assimilators now check that the assimilators address is not the zeroth address.

Description

For every method that allows to selectively withdraw, deposit, or swap tokens in Loihi, the user is allowed to specify addresses for the assimilators of said tokens (by inputting the addresses of the tokens themselves).

The shell then performs a lookup on a mapping called assimilators inside its main structure and uses the result of that lookup to delegate call the assimilator deployed by the shell administrator.

However, there are no checks for prior instantiation of a specific, supported token, effectively meaning that we can do a lookup on an all-zeroed-out member of that mapping and delegate call execution to the zeroth address.

The only thing preventing execution from going forward in this unwanted fashion is the fact that the ABI decoder expects a certain return data size from the interface implemented in Assimilator.sol.

For example, the 32 bytes expected as a result of this call:

src/Assimilators.sol:L58-L66

function viewNumeraireAmount (address _assim, uint256 _amt) internal returns (int128 amt_) {

    // amount_ = IAssimilator(_assim).viewNumeraireAmount(_amt); // for production

    bytes memory data = abi.encodeWithSelector(iAsmltr.viewNumeraireAmount.selector, _amt); // for development

    amt_ = abi.decode(_assim.delegate(data), (int128)); // for development

}

This is definitely an insufficient check since the interface for the assimilators might change in the future to include functions that have no return values.

Recommendation

Check for the prior instantiation of assimilators by including the following requirement:

require(shell.assimilators[<TOKEN_ADDRESS>].ix != 0);

In all the functions that access the assimilators mapping and change the indexes to be 1-based instead pf 0-based.

6.7 Math library’s fork has problematic changes Medium ✓ Fixed

Description

The math library ABDK Libraries for Solidity was forked and modified to add a few unsafe_* functions.

  • unsafe_add
  • unsafe_sub
  • unsafe_mul
  • unsafe_div
  • unsafe_abs

The problem which was introduced is that unsafe_add ironically is not really unsafe, it is as safe as the original add function. It is, in fact, identical to the safe add function.

src/ABDKMath64x64.sol:L102-L113

/**
 * Calculate x + y.  Revert on overflow.
 *
 * @param x signed 64.64-bit fixed point number
 * @param y signed 64.64-bit fixed point number
 * @return signed 64.64-bit fixed point number
 */
function add (int128 x, int128 y) internal pure returns (int128) {
  int256 result = int256(x) + y;
  require (result >= MIN_64x64 && result <= MAX_64x64);
  return int128 (result);
}

src/ABDKMath64x64.sol:L115-L126

/**
 * Calculate x + y.  Revert on overflow.
 *
 * @param x signed 64.64-bit fixed point number
 * @param y signed 64.64-bit fixed point number
 * @return signed 64.64-bit fixed point number
 */
function unsafe_add (int128 x, int128 y) internal pure returns (int128) {
  int256 result = int256(x) + y;
  require (result >= MIN_64x64 && result <= MAX_64x64);
  return int128 (result);
}

Fortunately, unsafe_add is not used anywhere in the code.

However, unsafe_abs was changed from this:

src/ABDKMath64x64.sol:L322-L331

/**
 * Calculate |x|.  Revert on overflow.
 *
 * @param x signed 64.64-bit fixed point number
 * @return signed 64.64-bit fixed point number
 */
function abs (int128 x) internal pure returns (int128) {
  require (x != MIN_64x64);
  return x < 0 ? -x : x;
}

To this:

src/ABDKMath64x64.sol:L333-L341

/**
 * Calculate |x|.  Revert on overflow.
 *
 * @param x signed 64.64-bit fixed point number
 * @return signed 64.64-bit fixed point number
 */
function unsafe_abs (int128 x) internal pure returns (int128) {
  return x < 0 ? -x : x;
}

The check that was removed, is actually an important check:

require (x != MIN_64x64);

src/ABDKMath64x64.sol:L19

int128 private constant MIN_64x64 = -0x80000000000000000000000000000000;

The problem is that for an int128 variable that is equal to -0x80000000000000000000000000000000, there is no absolute value within the constraints of int128.

Starting from int128 n = -0x80000000000000000000000000000000, the absolute value should be int128 abs_n = -n, however abs_n is equal to the initial value of n. The final value of abs_n is still -0x80000000000000000000000000000000. It’s still not a positive or zero value. The operation 0 - n wraps back to the same initial value.

Recommendation

Remove unused unsafe_* functions and try to find other ways of doing unsafe math (if it is fundamentally important) without changing existing, trusted, already audited code.

6.8 Use one file for each contract or library Medium ✓ Fixed

Resolution

Issue fixed by the development team.

Description

The repository contains a lot of contracts and libraries that are added in the same file as another contract or library.

Organizing the code in this manner makes it hard to navigate, develop and audit. It is a best practice to have each contract or library in its own file. The file also needs to bear the name of the hosted contract or library.

Examples

src/Shells.sol:L20

library SafeERC20Arithmetic {

src/Shells.sol:L32

library Shells {

src/Loihi.sol:L26-L28

contract ERC20Approve {
    function approve (address spender, uint256 amount) public returns (bool);
}

src/Loihi.sol:L30

contract Loihi is LoihiRoot {

src/Assimilators.sol:L19

library Delegate {

src/Assimilators.sol:L33

library Assimilators {

Recommendation

Split up contracts and libraries in single files.

6.9 Remove debugging code from the repository Medium ✓ Fixed

Resolution

Issue fixed but he development team.

Description

Throughout the repository, there is source code from the development stage that was used for debugging the functionality and was not removed.

This should not be present in the source code and even if they are used while functionality is developed, they should be removed after the functionality was implemented.

Examples

src/Shells.sol:L63-L67

event log(bytes32);
event log_int(bytes32, int256);
event log_ints(bytes32, int256[]);
event log_uint(bytes32, uint256);
event log_uints(bytes32, uint256[]);

src/Assimilators.sol:L44-L46

event log(bytes32);
event log_uint(bytes32, uint256);
event log_int(bytes32, int256);

src/Controller.sol:L33-L37

event log(bytes32);
event log_int(bytes32, int128);
event log_int(bytes32, int);
event log_uint(bytes32, uint);
event log_addr(bytes32, address);

src/LoihiRoot.sol:L53

event log(bytes32);

src/Shells.sol:L63-L67

event log(bytes32);
event log_int(bytes32, int256);
event log_ints(bytes32, int256[]);
event log_uint(bytes32, uint256);
event log_uints(bytes32, uint256[]);

src/Loihi.sol:L470-L474

event log_int(bytes32, int);
event log_ints(bytes32, int128[]);
event log_uint(bytes32, uint);
event log_uints(bytes32, uint[]);
event log_addrs(bytes32, address[]);

src/assimilators/mainnet/cdaiReserves/mainnetDaiToCDaiAssimilator.sol:L35-L36

event log_uint(bytes32, uint256);
event log_int(bytes32, int256);

src/assimilators/mainnet/cusdcReserves/mainnetUsdcToCUsdcAssimilator.sol:L38

event log_uint(bytes32, uint256);

src/Loihi.sol:L51

shell.testHalts = true;

src/LoihiRoot.sol:L79-L83

function setTestHalts (bool _testOrNotToTest) public {

    shell.testHalts = _testOrNotToTest;

}

src/Shells.sol:L60

bool testHalts;

Recommendation

Remove the debug functionality at the end of the development cycle of each functionality.

6.10 Tests should not fail Medium ✓ Fixed

Resolution

Comment from the development team:

The failing tests are because we made minute changes to our present model (changes in applying the base fee - “epsilon”), so in a sense, rather than failing they just need updating. Many of them are also an artifact of architecting the tests in such a way that they can be run against arbitrary parameter sets - or in different “suites”.

Description

The role of the tests should be to make sure the application behaves properly. This should include positive tests (functionality that should be implemented) and negative tests (behavior stopped or limited by the application).

The test suite should pass 100% of the tests. After spending time with the development team, we managed to ask for the changes that allowed us to run the tests suite. This revealed that out of the 555 tests, 206 are failing. This staggering number does not allow us to check what the problem is and makes anybody running tests ignore them completely.

Tests should be an integral part of the codebase, and they should be considered as important (or even more important) than the code itself. One should be able to recreate the whole codebase by just having the tests.

Recommendation

Update tests in order for the whole of the test suite to pass.

6.11 Remove commented out code from the repository Medium ✓ Fixed

Description

Having commented out code increases the cognitive load on an already complex system. Also, it hides the important parts of the system that should get the proper attention, but that attention gets to be diluted.

There is no code that is important enough to be left commented out in a repository. Git branching should take care of having different code versions or diffs should show what was before.

If there is commented out code, this also has to be maintained; it will be out of date if other parts of the system are changed, and the tests will not pick that up.

The main problem is that commented code adds confusion with no real benefit. Code should be code, and comments should be comments.

Examples

Commented out code should be removed or dealt with in a separate branch that is later included in the master branch.

src/Assimilators.sol:L48-L56

function viewRawAmount (address _assim, int128 _amt) internal returns (uint256 amount_) {

    // amount_ = IAssimilator(_assim).viewRawAmount(_amt); // for production

    bytes memory data = abi.encodeWithSelector(iAsmltr.viewRawAmount.selector, _amt.abs()); // for development

    amount_ = abi.decode(_assim.delegate(data), (uint256)); // for development

}

src/Assimilators.sol:L58-L66

function viewNumeraireAmount (address _assim, uint256 _amt) internal returns (int128 amt_) {

    // amount_ = IAssimilator(_assim).viewNumeraireAmount(_amt); // for production

    bytes memory data = abi.encodeWithSelector(iAsmltr.viewNumeraireAmount.selector, _amt); // for development

    amt_ = abi.decode(_assim.delegate(data), (int128)); // for development

}

src/Assimilators.sol:L58-L66

function viewNumeraireAmount (address _assim, uint256 _amt) internal returns (int128 amt_) {

    // amount_ = IAssimilator(_assim).viewNumeraireAmount(_amt); // for production

    bytes memory data = abi.encodeWithSelector(iAsmltr.viewNumeraireAmount.selector, _amt); // for development

    amt_ = abi.decode(_assim.delegate(data), (int128)); // for development

}

src/Controller.sol:L99-L106

function includeAssimilator (Shells.Shell storage shell, address _numeraire, address _derivative, address _assimilator) internal {

    Assimilators.Assimilator storage _numeraireAssim = shell.assimilators[_numeraire];

    shell.assimilators[_derivative] = Assimilators.Assimilator(_assimilator, _numeraireAssim.ix);
    // shell.assimilators[_derivative] = Assimilators.Assimilator(_assimilator, _numeraireAssim.ix, 0, 0);

}

src/Loihi.sol:L596-L618

function transfer (address _recipient, uint256 _amount) public nonReentrant returns (bool) {
    // return shell.transfer(_recipient, _amount);
}

function transferFrom (address _sender, address _recipient, uint256 _amount) public nonReentrant returns (bool) {
    // return shell.transferFrom(_sender, _recipient, _amount);
}

function approve (address _spender, uint256 _amount) public nonReentrant returns (bool success_) {
    // return shell.approve(_spender, _amount);
}

function increaseAllowance(address _spender, uint256 _addedValue) public returns (bool success_) {
    // return shell.increaseAllowance(_spender, _addedValue);
}

function decreaseAllowance(address _spender, uint256 _subtractedValue) public returns (bool success_) {
    // return shell.decreaseAllowance(_spender, _subtractedValue);
}

function balanceOf (address _account) public view returns (uint256) {
    // return shell.balances[_account];
}

src/test/deposits/suiteOne.t.sol:L15-L29

// function test_s1_selectiveDeposit_noSlippage_balanced_10DAI_10USDC_10USDT_2p5SUSD_NO_HACK () public logs_gas {

//     uint256 newShells = super.noSlippage_balanced_10DAI_10USDC_10USDT_2p5SUSD();

//     assertEq(newShells, 32499999216641686631);

// }

// function test_s1_selectiveDeposit_noSlippage_balanced_10DAI_10USDC_10USDT_2p5SUSD_HACK () public logs_gas {

//     uint256 newShells = super.noSlippage_balanced_10DAI_10USDC_10USDT_2p5SUSD_HACK();

//     assertEq(newShells, 32499999216641686631);

// }

src/test/deposits/depositsTemplate.sol:L40-L56

// function noSlippage_balanced_10DAI_10USDC_10USDT_2p5SUSD_HACK () public returns (uint256 shellsMinted_) {

//     uint256 startingShells = l.proportionalDeposit(300e18);

//     uint256 gas = gasleft();

//     shellsMinted_ = l.depositHack(
//         address(dai), 10e18,
//         address(usdc), 10e6,
//         address(usdt), 10e6,
//         address(susd), 2.5e18
//     );

//     emit log_uint("gas for deposit", gas - gasleft());


// }

Recommendation

Remove all the commented out code or transform it into comments.

6.12 Should check if the asset already exists when adding a new asset Medium ✓ Fixed

Resolution

Comment from the development team:

We have decided not to have dynamic adding/removing of assets in this release.

Description

The public function includeAsset

src/Loihi.sol:L128-L130

function includeAsset (address _numeraire, address _nAssim, address _reserve, address _rAssim, uint256 _weight) public onlyOwner {
    shell.includeAsset(_numeraire, _nAssim, _reserve, _rAssim, _weight);
}

Calls the internal includeAsset implementation

src/Controller.sol:L72

function includeAsset (Shells.Shell storage shell, address _numeraire, address _numeraireAssim, address _reserve, address _reserveAssim, uint256 _weight) internal {

But there is no check to see if the asset already exists in the list. Because the check was not done, shell.numeraires can contain multiple identical instances.

src/Controller.sol:L80

shell.numeraires.push(_numeraireAssimilator);

Recommendation

Check if the _numeraire already exists before invoking includeAsset.

6.13 Check return values for both internal and external calls Minor ✓ Fixed

Resolution

Comment from the development team:

This doesn’t seem feasible. Checking how much was transferred to/from the contract would pose unacceptable gas costs. Because of these constraints, the value returned by the assimilator methods never touches the outside world. They are just converted into numeraire format and returned, so checking these values would not provide any previously unknown information.

Description

There are some cases where functions which return values are called throughout the source code but the return values are not processed, nor checked.

The returns should in principle be handled and checked for validity to provide more robustness to the code.

Examples

The function intakeNumeraire receives a number of tokens and returns how many tokens were transferred to the contract.

src/assimilators/mainnet/daiReserves/mainnetDaiToDaiAssimilator.sol:L51-L59

// transfers numeraire amount of dai in, wraps it in cDai, returns raw amount
function intakeNumeraire (int128 _amount) public returns (uint256 amount_) {

    // truncate stray decimals caused by conversion
    amount_ = _amount.mulu(1e18) / 1e3 * 1e3;

    dai.transferFrom(msg.sender, address(this), amount_);

}

Similarly, the function outputNumeraire receives a destination address and an amount of token for withdrawal and returns a number of transferred tokens to the specified address.

src/assimilators/mainnet/daiReserves/mainnetDaiToDaiAssimilator.sol:L83-L92

// takes numeraire amount of dai, unwraps corresponding amount of cDai, transfers that out, returns numeraire amount
function outputNumeraire (address _dst, int128 _amount) public returns (uint256 amount_) {

    amount_ = _amount.mulu(1e18);

    dai.transfer(_dst, amount_);

    return amount_;

}

However, the results are not handled in the main contract.

src/Loihi.sol:L497

shell.numeraires[i].addr.intakeNumeraire(_shells.mul(shell.weights[i]));

src/Loihi.sol:L509

shell.numeraires[i].addr.intakeNumeraire(_oBals[i].mul(_multiplier));

src/Loihi.sol:L586

shell.reserves[i].addr.outputNumeraire(msg.sender, _oBals[i].mul(_multiplier));

A sanity check can be done to make sure that more than 0 tokens were transferred to the contract.

unit intakeAmount = shell.numeraires[i].addr.intakeNumeraire(_shells.mul(shell.weights[i]));
require(intakeAmount > 0, "Must intake a positive number of tokens");

Recommendation

Handle all return values everywhere returns exist and add checks to make sure an expected value was returned.

If the return values are never used, consider not returning them at all.

6.14 Interfaces do not need to be implemented for the compiler to access their selectors. Minor ✓ Fixed

Resolution

Comment from the development team:

This is the case for the version we used, solc 0.5.15. Versions 0.5.17 and 0.6.* do not require it.

Description

In Assimilators.sol the interface for the assimilators is implemented in a state variable constant as an interface to the zeroth address in order to make use of it’s selectors.

src/Assimilators.sol:L37

IAssimilator constant iAsmltr = IAssimilator(address(0));

This pattern is unneeded since you can reference selectors by using the imported interface directly without any implementation. It hinders both gas costs and readability of the code.

Examples

Recommendation

Delete line 37 in Assimilators.sol and instead of getting selectors through:

src/Assimilators.sol:L62

bytes memory data = abi.encodeWithSelector(iAsmltr.viewNumeraireAmount.selector, _amt); // for development

use the expression:

IAssimilator.viewRawAmount.selector

6.15 Use consistent interfaces for functions in the same group Minor ✓ Fixed

Description

In the file Shells.sol, there also is a library that is being used internally for safe adds and subtractions.

This library has 2 functions.

add which receives 2 arguments, x and y.

src/Shells.sol:L22-L24

function add(uint x, uint y) internal pure returns (uint z) {
    require((z = x + y) >= x, "add-overflow");
}

sub which receives 3 arguments x, y and _errorMessage.

src/Shells.sol:L26-L28

function sub(uint x, uint y, string memory _errorMessage) internal pure returns (uint z) {
    require((z = x - y) <= x, _errorMessage);
}

In order to reduce the cognitive load on the auditors and developers alike, somehow-related functions should have coherent logic and interfaces. Both of the functions either need to have 2 arguments, with an implied error message passed to require, or both functions need to have 3 arguments, with an error message that can be specified.

Recommendation

Update the functions to be coherent with other related functions.

6.16 Code coverage should be close to 100% Minor ✓ Fixed

Resolution

Comment from the development team:

This is true for all aspects of the bonding curve.

Things that have been tested on Kovan with the frontend dapp but could use a unit test are things relevant to sending shell tokens - issuing approvals, transfers and transferfroms.

The adding of assets and assimilators are tested by proxy because they are dependencies for the entire behavior of the bonding surface.

For this release, we plan on having the assets and the assimilators frozen at launch, so there is not much to test regarding continuous updating/changing of assets and assimilators.

We have, however, considered allowing for the dynamic adjustment of weights in addition to parameters.

Description

Code coverage is a measure used to describe how much of the source code is executed during the automated test suite. A system with high code coverage, measured as lines of code executed, has a lower chance to contain undiscovered bugs.

The codebase does not have any information about the code coverage.

Recommendation

Make the test suite output code coverage and add more tests to handle the lines of code that are not touched by any tests.

6.17 Consider emitting an event when changing the frozen state of the contract Minor ✓ Fixed

Description

The function freeze allows the owner to freeze and unfreeze the contract.

src/Loihi.sol:L144-L146

function freeze (bool _freeze) public onlyOwner {
    frozen = _freeze;
}

The common pattern when doing actions important for the outside of the blockchain is to emit an event when the action is successful.

It’s probably a good idea to emit an event stating the contract was frozen or unfrozen.

Recommendation

Create an event that displays the current state of the contract.

event Frozen(bool frozen);

And emit the event when frozen is called.

function freeze (bool _freeze) public onlyOwner {
    frozen = _freeze;
    emit Frozen(_freeze);
}

6.18 Function supportsInterface can be restricted to pure Minor ✓ Fixed

Description

The function supportsInterface returns a bool stating that the contract supports one of the defined interfaces.

src/Loihi.sol:L140-L142

function supportsInterface (bytes4 interfaceID) public returns (bool) {
    return interfaceID == ERC20ID || interfaceID == ERC165ID;
}

The function does not access or change the state of the contract, this is why it can be restricted to pure.

Recommendation

Restrict the function definition to pure.

function supportsInterface (bytes4 interfaceID) public pure returns (bool) {

6.19 Use more consistent function naming (includeAssimilator / excludeAdapter) Minor ✓ Fixed

Description

The function includeAssimilator adds a new assimilator to the list

src/Controller.sol:L98

shell.assimilators[_derivative] = Assimilators.Assimilator(_assimilator, _numeraireAssim.ix);

The function excludeAdapter removes the specified assimilator from the list

src/Loihi.sol:L137

delete shell.assimilators[_assimilator];

Recommendation

Consider renaming the function excludeAdapter to removeAssimilator and moving the logic of adding and removing in the same source file.

Appendix 1 - Files in Scope

This audit covered the following files:

File Name SHA-1 Hash
src/Assimilators.sol 3f6cc11fc01be7d858de29255ff2dcd7c73535a3
src/Controller.sol 96fefe583cf31c7ef45f2094367ae1527ed1fa3e
src/Loihi.sol de9feda8b31fae8494b5ea995d898be3251431a2
src/LoihiRoot.sol e2b21cdab22c7a42cc7ff03e5b202d67cc6c8d04
src/Shells.sol 2ae89c49fcec7d83aef5f7f0d95bd9e17d9efacb
src/ShellsExternal.sol becc7634a4bf45d08060be2fcb5e01382b6f8d4f
src/assimilators/AssimilatorMath.sol c4dfe2367edb23dab938d50d57a17fd5bb4c94b2
src/assimilators/aaveResources/ILendingPool.sol fe26c09c3be97a5bb37de95aa4ae895c948da251
src/assimilators/aaveResources/ILendingPoolAddressesProvider.sol 0f845e0d8d8456a963ce2717bdbccf27f58a4bf2
src/assimilators/mainnet/asusdReserves/mainnetASusdToASusdAssimilator.sol e1d56000137d13db62abafbc240a24f943ace70b
src/assimilators/mainnet/asusdReserves/mainnetSUsdToASUsdAssimilator.sol 35e8dbbb137e70a36598a8f783e396d9e8d0e5c5
src/assimilators/mainnet/ausdtReserves/mainnetAUsdtToAUsdtAssimilator.sol ea16a1544b169760821fed668bebee52ef99e72b
src/assimilators/mainnet/ausdtReserves/mainnetUsdtToAUsdtAssimilator.sol 495334fcb505cc45a628ca332438feb66183c772
src/assimilators/mainnet/cdaiReserves/mainnetCDaiToCDaiAssimilator.sol c268af639fe6e862917a5995ab1045222b325a03
src/assimilators/mainnet/cdaiReserves/mainnetChaiToCDaiAssimilator.sol ab2dc7613ac8b0dd4a44b19b643fc4c650711694
src/assimilators/mainnet/cdaiReserves/mainnetDaiToCDaiAssimilator.sol 0794a05a05356c73575da70ffa30d595ca53162f
src/assimilators/mainnet/cusdcReserves/mainnetCUsdcToCUsdcAssimilator.sol 4fbc3fc0b9fe2117460741bfeff80e80252afa51
src/assimilators/mainnet/cusdcReserves/mainnetUsdcToCUsdcAssimilator.sol 88e582d815fc08d34de014827a2ff4ec93f29292
src/assimilators/mainnet/daiReserves/mainnetCDaiToDaiAssimilator.sol 4a7d6eec1e609eb94590e2c37db80fc4fc5ea4ab
src/assimilators/mainnet/daiReserves/mainnetChaiToDaiAssimilator.sol 17b65aa02c02bf7ae98b2ca6d78892a99890ddb9
src/assimilators/mainnet/daiReserves/mainnetDaiToDaiAssimilator.sol 531f1c5c2982267eedcb9b52fa9f5dc611f5ae49
src/assimilators/mainnet/susdReserves/MainnetASusdToSUsdAssimilator.sol 444ff56afc3c610179976dcf56cbb8e6ce3029c0
src/assimilators/mainnet/susdReserves/MainnetSUsdToSUsdAssimilator.sol d33559f600a9a0a46c76a23b97c23b62a117c687
src/assimilators/mainnet/usdcReserves/localCUsdcToUsdcAssimilator.sol a63719169882e86a86620e3a505ef1e62f05d71c
src/assimilators/mainnet/usdcReserves/localUsdcToUsdcAssimilator.sol 95c5bc3b9470c74b0cc34a97c7f504d6ecb68033
src/assimilators/mainnet/usdtReserves/localAUsdtToUsdtAssimilator.sol 27c8b33955a9d6f043ab9ff9ff66fa9a916b0bc1
src/assimilators/mainnet/usdtReserves/localUsdtToUsdtAssimilator.sol 3da930b6f8d30210405e9f71189f2250b52a6287

Appendix 2 - Artifacts

This section contains some of the artifacts generated during our review by automated tools, the test suite, etc. If any issues or recommendations were identified by the output presented here, they have been addressed in the appropriate section above.

A.2.1 MythX

MythX is a security analysis API for Ethereum smart contracts. It performs multiple types of analysis, including fuzzing and symbolic execution, to detect many common vulnerability types. The tool was used for automated vulnerability discovery for all audited contracts and libraries. More details on MythX can be found at mythx.io.

The PDF report of the initial MythX vulnerability scan can be found here.

The PDF report for the followup MythX vulnerability scan, after code changes, can be found here.

A.2.2 Ethlint

Ethlint is an open source project for linting Solidity code. Only security-related issues were reviewed by the audit team.

Below is the raw output of the Ethlint vulnerability scan:

A.2.3 Surya

Surya is a utility tool for smart contract systems. It provides a number of visual outputs and information about the structure of smart contracts. It also supports querying the function call graph in multiple ways to aid in the manual inspection and control flow analysis of contracts.

Below is a complete list of functions with their visibility and modifiers:

A.2.4 Tests Suite

Below is the output generated by running the test suite:

Appendix 3 - Disclosure

ConsenSys Diligence (“CD”) typically receives compensation from one or more clients (the “Clients”) for performing the analysis contained in these reports (the “Reports”). The Reports may be distributed through other means, including via ConsenSys publications and other distributions.

The Reports are not an endorsement or indictment of any particular project or team, and the Reports do not guarantee the security of any particular project. This Report does not consider, and should not be interpreted as considering or having any bearing on, the potential economics of a token, token sale or any other product, service or other asset. Cryptographic tokens are emergent technologies and carry with them high levels of technical risk and uncertainty. No Report provides any warranty or representation to any Third-Party in any respect, including regarding the bugfree nature of code, the business model or proprietors of any such business model, and the legal compliance of any such business. No third party should rely on the Reports in any way, including for the purpose of making any decisions to buy or sell any token, product, service or other asset. Specifically, for the avoidance of doubt, this Report does not constitute investment advice, is not intended to be relied upon as investment advice, is not an endorsement of this project or team, and it is not a guarantee as to the absolute security of the project. CD owes no duty to any Third-Party by virtue of publishing these Reports.

PURPOSE OF REPORTS The Reports and the analysis described therein are created solely for Clients and published with their consent. The scope of our review is limited to a review of Solidity code and only the Solidity code we note as being within the scope of our review within this report. The Solidity language itself remains under development and is subject to unknown risks and flaws. The review does not extend to the compiler layer, or any other areas beyond Solidity that could present security risks. Cryptographic tokens are emergent technologies and carry with them high levels of technical risk and uncertainty.

CD makes the Reports available to parties other than the Clients (i.e., “third parties”) – on its website. CD hopes that by making these analyses publicly available, it can help the blockchain ecosystem develop technical best practices in this rapidly evolving area of innovation.

LINKS TO OTHER WEB SITES FROM THIS WEB SITE You may, through hypertext or other computer links, gain access to web sites operated by persons other than ConsenSys and CD. Such hyperlinks are provided for your reference and convenience only, and are the exclusive responsibility of such web sites’ owners. You agree that ConsenSys and CD are not responsible for the content or operation of such Web sites, and that ConsenSys and CD shall have no liability to you or any other person or entity for the use of third party Web sites. Except as described below, a hyperlink from this web Site to another web site does not imply or mean that ConsenSys and CD endorses the content on that Web site or the operator or operations of that site. You are solely responsible for determining the extent to which you may use any content at any other web sites to which you link from the Reports. ConsenSys and CD assumes no responsibility for the use of third party software on the Web Site and shall have no liability whatsoever to any person or entity for the accuracy or completeness of any outcome generated by such software.

TIMELINESS OF CONTENT The content contained in the Reports is current as of the date appearing on the Report and is subject to change without notice. Unless indicated otherwise, by ConsenSys and CD.