Security Audit
September 22, 2023
Version 1.0.0
Presented by 0xMacro
This document includes the results of the security audit for Endaoment's smart contract code as found in the section titled ‘Source Code’. The security audit was performed by the Macro security team from September 18, 2023 to September 19, 2023.
The purpose of this audit is to review the source code of certain Endaoment Solidity contracts, and provide feedback on the design, architecture, and quality of the source code with an emphasis on validating the correctness and security of the software in its entirety.
Disclaimer: While Macro’s review is comprehensive and has surfaced some changes that should be made to the source code, this audit should not solely be relied upon for security, as no single audit is guaranteed to catch all possible bugs.
The following is an aggregation of issues found by the Macro Audit team:
Severity | Count | Acknowledged | Won't Do | Addressed |
---|---|---|---|---|
Low | 1 | - | - | 1 |
Code Quality | 4 | - | 1 | 3 |
Gas Optimization | 3 | - | 2 | 1 |
Endaoment was quick to respond to these issues.
Our understanding of the specification was based on the following sources:
Endaoment is a nonprofit 501c3 and, as such, is legally required (through the Pension Protection Act of 2006) to have exclusive legal control over all contributions made to the system at any moment. Due to this, the contracts pose a centralization risk whereby the contract owner can shut down the portfolio; withdraw all assets; change the redemption, deposit, and AUM fees; and arbitrarily execute any function at any time. It is trusted that the owner will not abuse this power.
The following source code was reviewed during the audit:
13b8a2405574c29bfd1a88af76cc147ace4bb8ed
Specifically, we audited the following contracts within this repository:
Contract | SHA256 |
---|---|
src/portfolios/AaveUSDCPortfolio.sol |
|
src/portfolios/AaveV3USDCPortfolio.sol |
|
src/portfolios/CompoundUSDCPortfolio.sol |
|
src/portfolios/CompoundV3USDCPortfolio.sol |
|
src/portfolios/YearnUSDCPortfolio.sol |
|
src/Portfolio.sol |
|
src/lib/auth/EndaomentAuth.sol |
|
Note: This document contains an audit solely of the Solidity contracts listed above. Specifically, the audit pertains only to the contracts themselves, and does not pertain to any other programs or scripts, including deployment scripts.
Click on an issue to jump to it, or scroll down to see them all.
asset
checks in constructor
baseToken
rather than casting asset
to an ERC20
_maxCap()
baseToken
value in _checkCap()
We quantify issues in three parts:
This third part – the severity level – is a summary of how much consideration the client should give to fixing the issue. We assign severity according to the table of guidelines below:
Severity | Description |
---|---|
(C-x) Critical |
We recommend the client must fix the issue, no matter what, because not fixing would mean significant funds/assets WILL be lost. |
(H-x) High |
We recommend the client must address the issue, no matter what, because not fixing would be very bad, or some funds/assets will be lost, or the code’s behavior is against the provided spec. |
(M-x) Medium |
We recommend the client to seriously consider fixing the issue, as the implications of not fixing the issue are severe enough to impact the project significantly, albiet not in an existential manner. |
(L-x) Low |
The risk is small, unlikely, or may not relevant to the project in a meaningful way. Whether or not the project wants to develop a fix is up to the goals and needs of the project. |
(Q-x) Code Quality |
The issue identified does not pose any obvious risk, but fixing could improve overall code quality, on-chain composability, developer ergonomics, or even certain aspects of protocol design. |
(I-x) Informational |
Warnings and things to keep in mind when operating the protocol. No immediate action required. |
(G-x) Gas Optimizations |
The presented optimization suggestion would save an amount of gas significant enough, in our opinion, to be worth the development cost of implementing it. |
Currently Aave’s referral program is inactive for both V2 and V3 contracts, but could be re-implemented pending a governance vote. Currently in AaveUSDCPortfolio
and AaveV3USDCPortfolio
the referalCode
used is set to 0
, which indicates no referral, but in the case where the program starts back up, there may be a benefit to setup a referalCode
for Endaoment and use on deposits.
Remediations to Consider
Add a permissioned function that allows setting the referalCode
in case Endaoment would benefit from referring entities to use Aave.
asset
checks in constructor
In each of the yield bearing portfolio contracts, in their constructor there are two checks to ensure that the baseToken
is equal to the set asset
and the asset returned by _getAsset.
// The `asset` should match the base token, which means we expect it to be USDC. So we
// execute some checks to make sure inputs are consistent.
if (address(baseToken) != _getAsset(_receiptAsset)) revert AssetMismatch();
if (address(baseToken) != asset) revert AssetMismatch();
Reference: CompoundV3USDCPortfolio.sol#L52-L55
However, both are equivalent checks, as the value of asset
is set in the constructor of Portfolio.sol using the _getAsset()
function.
asset = _getAsset(_receiptAsset);
Reference: Portfolio.sol#L120
Remediations to Consider
In each of the yield bearing portfolio contracts, remove one of these checks, as they are duplicates.
baseToken
rather than casting asset
to an ERC20
Since there is a constraint for each of the yield bearing portfolio contracts that the asset
equals the baseToken
in their constructors, any call to transfer the asset
tokens can directly be called on baseToken
instead of casting asset
to an ERC20
.
Remediations to Consider
Replace casting asset
to an ERC20
and use baseToken
instead.
Casting
asset
is more idiomatic than usingbaseToken
directly
Endaoment is deployed on both Ethereum and Base, however the yield bearing portfolios use constant address values for the protocols they interact with that are deployed on Ethereum. In the case where a portfolio would want to be deployed on another chain, like Base, it would require a duplicate contract to be added to the repo with a differing protocol address, if the address of the protocol varies from the Ethereum version. If these addresses were immutable and initialized in the constructor, then these portfolio contracts could be deployed on any desired chain that the portfolio’s protocol supports.
Remediations to Consider
Replace constant protocol addresses with immutable values and initialize the values in their constructors.
In the following instances, the NatSpec documentation is incomplete:
YearnUSDCPortfolio
, convertToUsdc()
and convertToYvUsdc()
are missing @param
and @return
fieldsCompoundUSDCPortfolio
, compoundExchangeRateCurrent()
is missing the @return
fieldWhenever a entity either deposits or redeems, it invokes multiple ERC20 transfers to the treasury due to AUM fees as well as deposit/redemption fees. Each of these costs the entity extra gas to execute, and could be reduced.
Remediations to Consider
Portfolio could be reworked to transfer the sum of the AUM fees and the deposit/redeem fees to reduce the number of external transfer calls. Alternatively instead of transferring fees immediately, keep track of fees owed and allow a permissioned function to take the owed fees and reset the tracked value, while making sure to subtract the fees owed from any calculations relating to the total supply of funds in the portfolio, while ensuring rebasing tokens are handled appropriately. Doing either would reduce the gas cost for users interacting with the protocol.
Prefer separation of AUM and deposit/redemption code and fee assessment
_maxCap()
In Portfolio.sol
, the function _maxCap()
checks the registry to ensure the asset is the same as the registry’s baseToken.
function _checkCap() internal virtual {
if (asset != address(registry.baseToken())) revert BadCheckCapImplementation();
if (totalAssets() > cap) revert ExceedsCap();
}
Reference: Portfolio.sol#L295-L298
However, in each of the 5 portfolio contracts in scope, it is ensured in the constructor that the asset
is the same as the registry’s baseToken
, since both of which are immutable
, the check is unnecessary.
if (address(baseToken) != asset) revert AssetMismatch();
Reference: AaveUSDCPortfolio.sol#L56
Remediations to Consider
In each of the portfolio contracts in scope, override the _checkCap()
function, removing the unnecessary check, reducing gas costs.
Check was recommended in last audit, so we’ll leave it in place
baseToken
value in _checkCap()
In Portfolio.sol
, the _checkCap()
function queries the registry for the baseToken
and checks to see if the portfolios asset
is the same.
function _checkCap() internal virtual {
if (asset != address(registry.baseToken())) revert BadCheckCapImplementation();
if (totalAssets() > cap) revert ExceedsCap();
}
Reference: Portfolio.sol#L295-L298
However, the baseToken
value in the registry is immutable, so it cannot change, and in Portfolio.sol
’s constructor it sets the value of baseToken
by querying it from the registry, so the value of baseToken
will be the same as the value queried from the registry, but costs less gas to use as it isn’t an external call.
Remediations to Consider
Use baseToken
instead of registry.baseToken()
in _checkCap()
to prevent an external call, and reduce gas cost.
Macro makes no warranties, either express, implied, statutory, or otherwise, with respect to the services or deliverables provided in this report, and Macro specifically disclaims all implied warranties of merchantability, fitness for a particular purpose, noninfringement and those arising from a course of dealing, usage or trade with respect thereto, and all such warranties are hereby excluded to the fullest extent permitted by law.
Macro will not be liable for any lost profits, business, contracts, revenue, goodwill, production, anticipated savings, loss of data, or costs of procurement of substitute goods or services or for any claim or demand by any other party. In no event will Macro be liable for consequential, incidental, special, indirect, or exemplary damages arising out of this agreement or any work statement, however caused and (to the fullest extent permitted by law) under any theory of liability (including negligence), even if Macro has been advised of the possibility of such damages.
The scope of this report and review is limited to a review of only the code presented by the Endaoment team and only the source code Macro notes as being within the scope of Macro’s review within this report. This report does not include an audit of the deployment scripts used to deploy the Solidity contracts in the repository corresponding to this audit. 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. In this report you may through hypertext or other computer links, gain access to websites operated by persons other than Macro. Such hyperlinks are provided for your reference and convenience only, and are the exclusive responsibility of such websites’ owners. You agree that Macro is not responsible for the content or operation of such websites, and that Macro shall have no liability to your or any other person or entity for the use of third party websites. Macro assumes no responsibility for the use of third party software and shall have no liability whatsoever to any person or entity for the accuracy or completeness of any outcome generated by such software.