Omniscia Tangible Audit
BasketBeaconProxy Manual Review Findings
BasketBeaconProxy Manual Review Findings
BBP-01M: Proxy Signature Incompatibility
Type | Severity | Location |
---|---|---|
Language Specific | BasketBeaconProxy.sol:L30, L37 |
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:
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. This17 * will typically be an encoded function call, and allows initializing the storage of the proxy like a Solidity18 * 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.