meetm on Warpcast

Content pfp

0 reply

0 recast

2 reactions

bbuddha pfp

Are there any existing implementations of Gnosis Safe that restrict the ability change its threshold and signers to a configurable address rather than the safe itself?

2 replies

0 recast

3 reactions

meetm pfp

Hard to restrict anything and cover all tracks as long as it has modules and delegate calls. First step would be to strip those out.

2 replies

0 recast

3 reactions

Spencer Graham 🧢 pfp

Spencer Graham 🧢

Its tough (and adds gas overhead) but you can use the guard hook to check for and prevent those types of changes. This is what we did for Hats Signer Gate: https://github.com/Hats-Protocol/hats-zodiac#hats-signer-gate

1 reply

0 recast

0 reaction

meetm pfp

Not particularly familiar with zodiac. But guards don't execute on module transactions.

1 reply

0 recast

0 reaction

Spencer Graham 🧢 pfp

Spencer Graham 🧢

That's true, but in our case the signers initiate txs via the normal Safe.exec() path, not from the module. The "only" thing module does is manage who can be a signer, sets the threshold appropriately, and constrains the signers' txs accordingly.

1 reply

0 recast

0 reaction

meetm pfp

Looking at the gate seems like owners can get out of sync if no one synced it after the hats was revoked?

1 reply

0 recast

0 reaction

Spencer Graham 🧢 pfp

Spencer Graham 🧢

True, though... 1. anybody can remove an invalid signer 2. the guard validates signatures based on current hat wearer status, so signatures from invalid signers are rejected

1 reply

0 recast

0 reaction

meetm pfp

1 reply

0 recast

1 reaction

meetm pfp

but yeah its hard to guarantee anything on a safe as long as delegatecalls exist

1 reply

0 recast

1 reaction

meetm pfp

best you can do is add a guard before any modules are added and prevent adding modules with the guard thereafter. but even then there is no way for guard to know if no modules existed previously by direct storage write to the module slot

1 reply

0 recast

1 reaction

meetm pfp

checking now though it seems like next version of safe will execute guards even on module transactions https://github.com/safe-global/safe-contracts/commit/426e965d6533e1d1e9a2cc8830cdfa1b868a08da

1 reply

0 recast

1 reaction

meetm pfp

direct storage writes to mapping slots from delegatecalls are still not protected against and will never be until delegatecalls are turned off completely

1 reply

0 recast

1 reaction

Spencer Graham 🧢 pfp

Spencer Graham 🧢

not sure i'm following here... can't you write guard logic to explicitly check for such changes?

1 reply

0 recast

0 reaction

meetm pfp

you cant. consider a delegatecall to a contract X. the guard wont see it as a `enableModule` transaction so it will let it pass. and this will set the modules[module] slot. https://github.com/safe-global/safe-contracts/blob/main/contracts/base/ModuleManager.sol#L89 essentially making it a module.

2 replies

0 recast

0 reaction

Spencer Graham 🧢 pfp

Spencer Graham 🧢

this is using the checkTransaction() guard function, which is limited to parsing tx inputs and so is easily bypassed as you describe. But you can also use the checkAfterExecution() guard function, which checks the *effects* of the tx.

1 reply

0 recast

0 reaction

meetm pfp

what slots would you check? how many slots would you check? they can be anything

2 replies

0 recast

1 reaction

meetm pfp

even if there was some opcode that provided you with list of storage slot diffs you still cant tell if a particular slot was a module mapping slot because of the slot being a hash of the key, etc.

0 reply

0 recast

1 reaction

Spencer Graham 🧢 pfp

Spencer Graham 🧢

HSG handles this today by calling getModulesPaginated() and enforcing that the length is 1 but that assumes that any module added during the tx has been added to the module linked list (which is what getModulesPaginated uses, which as you point out is not guaranteed

1 reply

0 recast

0 reaction