AsProxy Manual Review Findings

APY-01M: Reservation of Function Signatures

Type	Severity	Location
Standard Conformity		AsProxy.sol:L57, L65, L73

Description:

Any Proxy implementation is meant to relay calls to its logic contract and should not implement any functions of its own to avoid function signature clashes (i.e. a function signature being present in both the Proxy and its logic implementation).

In such cases, the function signature in the Proxy implementation will take precedence preventing the function from the logic contract from ever being invoked via it.

Impact:

The probability of a function signature collision is low but not unlikely given that only 4 bytes are utilized of the resulting function's hash. As such, it is advised that these implementations are instead present in the logic contract to ensure that the proxy is a pass-through contract rather than one with logic within it.

Example:

Copy
54/**
55 * @notice Returns the proxy initialization state
56 */
57function initialized() public view virtual returns (bool) {
58    return _implementation() != address(0);
59}
60
61/**
62 * @dev Returns the EIP-897 address of the implementation contract
63 * @return The address of the implementation contract
64 */
65function implementation() external view virtual returns (address) {
66    return _implementation();
67}
68
69/**
70 * @dev Returns the EIP-897 proxy type
71 * @return The proxy type
72 */
73function proxyType() external pure virtual returns (uint256) {
74    return 2;
75}

Recommendation:

We advise the functions to be implemented by the logic implementation instead, ensuring that all function signatures are properly forwarded to the logic contract.

Alleviation (59b75fbee1d8f3dee807c928f18be41c58b904e1):

The AsProxy implementation was removed from the codebase after consideration of the audit report's outputs and its usage has been replaced by vanilla delegatecall integrations.

As such, all exhibits relevant to it have been marked as no longer applicable.

APY-02M: Potentially Insecure Utilization of Scratch Space

Type	Severity	Location
Language Specific		AsProxy.sol:L29, L31

Description:

The AsProxy::_delegateWithSignature function attempts to mimic the Proxy::_delegate implementation by taking control over the full memory scratch space whose security relies entirely on the way the function is invoked as well as the primitives that are used around its invocation.

Impact:

A severity of minor has been assigned due to the fact that the top-level call that leads to the AsProxy::_delegateWithSignature function's execution has been confirmed as being the final statement in each code block.

In spite of this, we still advise proper memory reservation to occur as it represents a somewhat small gas increase while significantly bolstering the security of these relayed calls.

Example:

Copy
17/**
18 * @notice Delegate a call to an implementation contract using a function signature
19 * @param _implementation The address of the implementation contract
20 * @param _signature The function signature to delegate
21 */
22function _delegateWithSignature(
23    address _implementation,
24    string memory _signature
25) internal {
26    bytes4 selector = bytes4(keccak256(bytes(_signature)));
27    assembly {
28        // Store selector at the beginning of the calldata
29        mstore(0x0, selector)
30        // Copy the rest of calldata (skipping the first 4 bytes of the original function signature)
31        calldatacopy(0x4, 0x4, sub(calldatasize(), 0x4))
32        let result := delegatecall(
33            gas(),
34            _implementation,
35            0x0,
36            calldatasize(),
37            0,
38            0
39        )
40        let size := returndatasize()
41        let ptr := mload(0x40)
42        returndatacopy(ptr, 0, size)
43
44        switch result
45        case 0 {
46            revert(ptr, size)
47        }
48        default {
49            return(ptr, size)
50        }
51    }
52}

Recommendation:

We strongly advise against utilizing the scratch space based on the fact that the AsProxy::_delegateWithSignature function is invoked within other functions, the usage of keccak256 prior to the assembly block which utilizes the scratch space itself, and the fact that the memory required by the function is dynamic and reliant on the call-data of the top-level call.

Alleviation (59b75fbee1d8f3dee807c928f18be41c58b904e1):

The AsProxy implementation was removed from the codebase after consideration of the audit report's outputs and its usage has been replaced by vanilla delegatecall integrations.

As such, all exhibits relevant to it have been marked as no longer applicable.

APY-03M: Insecure Forwarded Payload

Type	Severity	Location
Logical Fault		AsProxy.sol:L31

Description:

The AsProxy::_delegateWithSignature function will forward the payload attached to the transaction's calldata to the selector associated with the input function _signature of the function.

Based on the way the AsProxy::_delegateWithSignature function is invoked within the codebase, the relayed payload will be outright incorrect or contain superfluous data points in the following cases:

• StrategyV5Chainlink::updateAsset: The _feed of the function will be passed in as the rate (i.e. _priceFactor) which is invalid.
• StrategyV5Pyth::updateAsset: The _feed of the function will be passed in as the rate (i.e. _priceFactor) which is invalid.
• StrategyV5Chainlink::setInputs: The function will relay the _feeds and _validities arrays redundantly.
• StrategyV5Pyth::setInputs: The function will relay the _feeds and _validities arrays redundantly.
• StrategyV5Chainlink::_init: The function call will relay the ChainlinkParams and Params payloads that are part of the top-level calls redundantly.
• StrategyV5Pyth::_init: The function call will relay the PythParams and Params payloads that are part of the top-level calls redundantly.

Impact:

Any StrategyV5Chainlink::updateAsset / StrategyV5Pyth::updateAsset call will result in misbehaviour as it will relay an improper _priceFactor which we consider a significant misbehaviour.

Example:

Copy
17/**
18 * @notice Delegate a call to an implementation contract using a function signature
19 * @param _implementation The address of the implementation contract
20 * @param _signature The function signature to delegate
21 */
22function _delegateWithSignature(
23    address _implementation,
24    string memory _signature
25) internal {
26    bytes4 selector = bytes4(keccak256(bytes(_signature)));
27    assembly {
28        // Store selector at the beginning of the calldata
29        mstore(0x0, selector)
30        // Copy the rest of calldata (skipping the first 4 bytes of the original function signature)
31        calldatacopy(0x4, 0x4, sub(calldatasize(), 0x4))
32        let result := delegatecall(
33            gas(),
34            _implementation,
35            0x0,
36            calldatasize(),
37            0,
38            0
39        )
40        let size := returndatasize()
41        let ptr := mload(0x40)
42        returndatacopy(ptr, 0, size)
43
44        switch result
45        case 0 {
46            revert(ptr, size)
47        }
48        default {
49            return(ptr, size)
50        }
51    }
52}

Recommendation:

The flaw arises from the fact that the transaction's calldata is utilized, and the calldata remains the same regardless of how many internal functions are invoked as only an external call can mutate the calldata.

As the function is never used to actually forward a dynamic calldata based payload, we advise a bytes memory argument to be introduced to the function that is in turn forwarded, ensuring that the data the _implementation contract receives is accurate and expectable.

Alleviation (59b75fbee1d8f3dee807c928f18be41c58b904e1):

The AsProxy implementation was removed from the codebase after consideration of the audit report's outputs and its usage has been replaced by vanilla delegatecall integrations.

As such, all exhibits relevant to it have been marked as no longer applicable.