Missing `threshold` check in `removeOwner`

[hats-bug-reporter[bot]]

Github username: @Jelev123 Twitter username: zhulien_zhelev Submission hash (on-chain): 0x43348c25efcebf59a99aad312198fcf8c7fd12a6adcf6de264c058aa14688eed Severity: low

Description: Description\ The removeOwner function in the PalmeraModule.sol contract allows the caller to remove a new owner and safe and set a threshold for the safe multisig wallet. But the function does not currently validate that the threshold parameter is greater than 0. This could lead to the threshold being set to 0 which may cause unexpected behavior. Attack Scenario\ Describe how the vulnerability can be exploited.

Impact Managing the Safe's configuration can be challenging due to potential inconsistencies. The multisig logic of the Safe might malfunction, causing operational problems and possibly resulting in the loss of funds.

1. Proof of Concept (PoC) File

https://github.com/keyper-labs/PalmeraModule/blob/dfd821e2fd7825c66c079c19be9460238f6e045a/src/PalmeraModule.sol#L235

Recommendation

In the removeOwner function, a validation check should be implemented to confirm that the threshold parameter is greater than 0. This can be accomplished by inserting a straightforward require statement at the start of the function.

[0xRizwan]

Centralized issue, should be invalid.

[Jelev123]

what is that mean the issue is centralized

[alfredolopez80]

Non-Issue, i agree with @0xRizwan, because safe check that when Palmera Module send the tx with _executeModuleTransaction, in the args data called the same way removeOwner, If we add this require, it would be a redundant verification which is absolutely inefficient at the gas level and bytesize code.

Verification into Safe-Contracts: https://github.com/safe-global/safe-smart-account/blob/186a21a74b327f17fc41217a927dea7064f74604/contracts/base/OwnerManager.sol#L121