Omniscia Tangible Audit

BasketBeaconProxy Manual Review Findings

BasketBeaconProxy Manual Review Findings

BBP-01M: Proxy Signature Incompatibility

Description:

Due to how the bytecode of a smart contract is generated, a proxy meant to relay calls via a fallback function that has explicit functions defined would cause the explicit functions to take precedence over the fallback function, thereby never relaying those function signatures.

Example:

src/proxy/beacon/BasketBeaconProxy.sol
7/**
8 * @dev This is a custom contract. It adds 2 helper methods to the BeaconProxy pattern.
9 * This makes testing state easier when we're able to pull beacon data from the proxy.
10 */
11contract BasketBeaconProxy is BeaconProxy {
12
13 /**
14 * @dev Initializes the proxy with `beacon`.
15 *
16 * If `data` is nonempty, it's used as data in a delegate call to the implementation returned by the beacon. This
17 * will typically be an encoded function call, and allows initializing the storage of the proxy like a Solidity
18 * constructor.
19 *
20 * Requirements:
21 *
22 * - `beacon` must be a contract with the interface {IBeacon}.
23 * - If `data` is empty, `msg.value` must be zero.
24 */
25 constructor(address beacon, bytes memory data) BeaconProxy(beacon, data) payable {}
26
27 /**
28 * @dev This method is mainly for testing purposes. Allows us to fetch externally the implementation address from beacon.
29 */
30 function implementation() external view returns (address) {
31 return IBeacon(_getBeacon()).implementation();
32 }
33
34 /**
35 * @dev This method is mainly for testing purposes. Allows us to fetch externally the beacon address for this beacon proxy.
36 */
37 function getBeacon() external view returns (address) {
38 return _getBeacon();
39 }
40}

Recommendation:

In the case of the BasketBeaconProxy we have confirmed that the BasketBeaconProxy::implementation (0x5c60da1b) and BasketBeaconProxy::getBeacon (0x2d6b3a6b) function signatures do not collide with any of the Basket functions, however, this caveat should be taken into account for future upgrades of the Basket as a function of it expected to be accessible via the BasketBeaconProxy may not be so.

Alleviation (106fc61dcd38fe29a81d1984ccde9171f5f231af):

The Tangible team proceeded with removing the referenced functions to avoid a clash such as the one described in this exhibit, alleviating it in full.