DefiSaver V3 Strategies

Smart Contract Security Assessment

Dec. 22, 2021

SUMMARY​

ABSTRACT​

Dedaub was commissioned to perform a security audit on Decenter’s DefiSaver V3 strategy smart contracts at commit hash 7b7b64d3a76ef9bdc3f6659a6721c96a5c65182f. The code can be found in the DeFi Saver v3 contracts Github repository. The scope of the audit, as defined by the client, includes the directories/files:

• core
• auth
• triggers, in terms of their base model and not detail
• ActionBase.sol
• actions/checkers
• actions/fee
• utils/TempStorage.sol.

We have audited earlier versions of the DeFi Saver contracts, but not the full Strategy/Recipe automation that is defined in the above code. We avoid repeating the main system architecture elements and caveats from previous audits, focusing instead on the audited subset.

SETTING & CAVEATS​

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. Typically it can only be covered if we are provided with unambiguous (i.e., full-detail) specifications of what is the expected, correct behavior. In terms of functional correctness, we often trusted the code’s calculations and interactions, in the absence of any other specification. 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.

The scope of this audit includes the contracts that handle Strategies: composite operations consisting of a number of triggers (preconditions), a number of modular actions, as well as a parameter mapping array that is used to assign action parameters from subscription data or the return value of a previous call. Related/alternative strategies can be combined into Bundles. Users can create Strategies and Bundles using the corresponding {Strategy|Bundle}Storage contract, which can be configured to either be open to anyone or limited to a single address. A user can subscribe to a Strategy or Bundle using the SubStorage contract by providing a StrategySub which includes the Strategy/Bundle id, triggerData, and subData (provided to actions), which is hashed and stored as part of the StoredSubData identifying the Subscription instance. To interface with the above mentioned *Storage contracts, the StrategyProxy and SubProxy contracts are provided for users to delegate-call into using their DSProxies.

Upon subscribing to a Strategy, users give the ProxyAuth contract permission to call the execute method of their DSProxy. The ProxyAuth contract can only do that when called by the StrategyExecutor contract which, in turn, only allows whitelisted bots to call its protected functionality.

The definition of a Strategy determines how much “freedom” a bot has when initiating the execution of a subscription instance/Recipe. For the Strategy’s triggers it can be deduced by inspecting their source code, with the current implementations favoring subscription data to bot-provided calldata (only the McdRatioTrigger makes use of the calldata). For the Strategy’s actions their code needs to be inspected together with the parameter mapping that is part of the Strategy definition.

The stateful contracts mentioned throughout this report are registered on the DFSRegistry contract which serves as the project’s central registry, allowing the project’s owner to add new entries/contracts and update them after a waiting period. References to intra-protocol contracts in most of the codebase are resolved using the DFSRegistry, making it a centralization element of the protocol. It should be noted that the waiting period is defined per-entry and it doesn’t have a hard-coded minimum value so it can be as small as the owner sets it to be.

In addition, most contracts audited are subcontracts of the AdminAuth, allowing the project’s administrators to easily retrieve stuck funds or destroy the contracts if such a need arises, without affecting their users in a negative way.

VULNERABILITIES & FUNCTIONAL ISSUES​

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

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

CRITICAL SEVERITY​

[No critical severity issues]

HIGH SEVERITY​

[No high severity issues]

MEDIUM SEVERITY​

[No medium severity issues]

LOW SEVERITY​

modifier isValidId(uint256 _id, bool _isBundle) {
 if (_isBundle) {
  if (_id > BundleStorage(registry.getAddr(BUNDLE_STORAGE_ID)).getBundleCount()) {
    revert SubIdOutOfRange(_id, _isBundle);
  }
 } else {
  if (_id > StrategyStorage(registry.getAddr(STRATEGY_STORAGE_ID)).
                                                            getStrategyCount()) {
    revert SubIdOutOfRange(_id, _isBundle);
  }
 }
 _;
}

function set(string memory _id, bytes32 _value) public {
   storageSlot[_id] = _value;
}

function get(string memory _id) public view returns (bytes32) {
   return storageSlot[_id];
}

OTHER/ ADVISORY ISSUES​

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

function mulUInt(uint a, uint b) internal pure returns (MathError, uint) {
	if (a == 0) {
        	return (MathError.NO_ERROR, 0);
    	}

    	uint c = a * b;

    	if (c / a != b) {
        	return (MathError.INTEGER_OVERFLOW, 0);
    	} else {
        	return (MathError.NO_ERROR, c);
    	}
}

DFSRegistry public constant registry = DFSRegistry(REGISTRY_ADDR);
bytes4 constant STRATEGY_STORAGE_ID = bytes4(keccak256("StrategyStorage"));
bytes4 constant SUB_STORAGE_ID = bytes4(keccak256("SubStorage"));
bytes4 constant BUNDLE_STORAGE_ID = bytes4(keccak256("BundleStorage"));

function getPaginatedStrategies(uint _page, uint _perPage) public view
returns (Strategy[] memory) {
    	Strategy[] memory strategiesPerPage = new Strategy[](_perPage);

    	uint start = _page * _perPage;
    	uint end = start + _perPage;

    	end = (end > strategies.length) ? strategies.length : end;

    	uint count = 0;
    	for (uint i = start; i < end; i++) {
        	strategiesPerPage[count] = strategies[i];
        	count++;
    	}

    	return strategiesPerPage;
}

if (RatioCheck(triggerCallData.ratioCheck) == RatioCheck.CURR_RATIO || RatioCheck(triggerCallData.ratioCheck) == RatioCheck.BOTH_RATIOS){
   checkedRatio = getRatio(triggerSubData.vaultId, 0);
   shouldTriggerCurr = ...
}

if (RatioCheck(triggerCallData.ratioCheck) == RatioCheck.NEXT_RATIO || RatioCheck(triggerCallData.ratioCheck) == RatioCheck.BOTH_RATIOS){
   checkedRatio = getRatio(triggerSubData.vaultId, triggerCallData.nextPrice);
   shouldTriggerNext = ...
}

TempStorage(tempStorageAddr).set("MCD_RATIO", bytes32(checkedRatio));

return shouldTriggerCurr || shouldTriggerNext;

function addNewContract(
   bytes4 _id,
   address _contractAddr,
   uint256 _waitPeriod
) public onlyOwner {
   if (entries[_id].exists){
       revert EntryAlreadyExistsError(_id);
   }

   entries[_id] = Entry({
       contractAddr: _contractAddr,
       ...
   });

   // Remember tha address so we can revert back to old addr if needed
   previousAddresses[_id] = _contractAddr;
   emit AddNewContract(msg.sender, _id, _contractAddr, _waitPeriod);
}

function revertToPreviousAddress(bytes4 _id) public onlyOwner {
   if (!(entries[_id].exists)){
     revert EntryNonExistentError(_id);
   }
   if (previousAddresses[_id] == address(0)){
     revert EmptyPrevAddrError(_id);
   }

   address currentAddr = entries[_id].contractAddr;
   entries[_id].contractAddr = previousAddresses[_id];

   emit RevertToPreviousAddress(msg.sender, _id, currentAddr, previousAddresses[_id]);
}

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.