Rhaydden - Missing update functionality for compatibilityFallback in setParam function

[sherlock-admin2]

Rhaydden

Medium

Missing update functionality for compatibilityFallback in setParam function

Summary

The RumpelWalletFactory contract has no ability to update the compatibilityFallback address after deployment. This could lead to issues if the fallback handler needs to be changed due to upgrades or bugs.

Vulnerability Detail

In the RumpelWalletFactory.sol, the compatibilityFallback address is set during the contract's construction. However, the setParam function, which allows the owner to update various parameters, does not include an option to update the compatibilityFallback address. This omission technically means that once the contract is deployed, the compatibilityFallback address cannot be changed.

Impact

This will render the contract useless. According to Sherlock docs:

How to identify a medium issue:

Breaks core contract functionality, rendering the contract useless or leading to loss of funds.

The inability to update the compatibilityFallback address could result in the need to redeploy the entire contract entirely if the fallback handler needs to be changed. This could be due to an upgrade, a bug in the fallback handler, or other issues.

Code Snippet

https://github.com/sherlock-audit/2024-07-sense-points-marketplace/blob/main/rumpel-wallet/src/RumpelWalletFactory.sol#L85-L93

Tool used

Manual Review

Recommendation

Add an option to update the compatibilityFallback address in the setParam function:

function setParam(bytes32 what, address data) external onlyOwner {
    if (what == "PROXY_FACTORY") proxyFactory = ISafeProxyFactory(data);
    else if (what == "SAFE_SINGLETON") safeSingleton = data;
    else if (what == "RUMPEL_MODULE") rumpelModule = data;
    else if (what == "RUMPEL_GUARD") rumpelGuard = data;
    else if (what == "INITIALIZATION_SCRIPT") initializationScript = data;
+    else if (what == "COMPATIBILITY_FALLBACK") compatibilityFallback = data; // Add this line
    else revert UnrecognizedParam(what);
    emit ParamChanged(what, data);
}

[Alireza-Razavi]

This seems to be low due to sponsor's comment:

This does not meet the criteria for a medium. There is an unlikely possibility that upgrading the singleton or safe factory would require also updating the compatibility fallback handler. however, this does not lead to loss of funds or make the contract useless. It just limits the flexibility of out future upgrade plans.

[jparklev]

PR fix: sense-finance/rumpel-wallet#5

[sherlock-admin2]

The protocol team fixed this issue in the following PRs/commits:
sense-finance/rumpel-wallet#5

[sherlock-admin2]

The Lead Senior Watson signed off on the fix.