Crucial functions are secured with tx-sender

[LNow]

It is not a bug per-se, but securing crucial functions with just tx-sender can be considered as security flaw.

Depending on your security strategy and approach I would suggest securing those functions with contract-caller or add 1uSTX transfer to crucial functions in order to enforce the need to construct TX with post-conditions.

Reference: https://github.com/LNow/clarity-notes/blob/main/security/function-calls.md

[talhasch]

In some cases it may be dangerous to protect functions with tx-sender

But in our case;

These are protected functions with tx-sender:

add-owner
remove-owner
set-min-confirmation

They all require to be triggered from the same smart contract to get executed.

as-contract directive on confirm function (here) switches tx context and and guarantees tx-sender is it's own principal.

So i believe this using this approach is not dangerous.

[LNow]

All true, however with current approach if you manage to trick safe owners to interact (directly or indirectly) with malicious contract it is possible to create and confirm transaction without their knowledge.
For example you can add new owner and change min-confirmation. And once you manage to do that, safe is fully compromised.

It's not an easy task to convince someone to interact with your contract, but for sure it is not impossible, therefore it should be taken into account.

[talhasch]

We implemented an access control to add an extra protection to function works with tx-sender with this commit: 954839b