`addAllowedRecipients()` Function Execution Manifest Skips All Validation, Allowing Anyone to Expand Any Account's Allowlist

[Schereo]

File affected:

buidl-wallet-contracts/src/msca/6900/v0.8/modules/addressbook/ColdStorageAddressBookModule.sol

Line 238 in 3c47aa9

skipRuntimeValidation: true,

Description
The executionManifest() function in the ColdStorageAddressBookModule contract configures the addAllowedRecipients() function with skipRuntimeValidation: true.

With skipRuntimeValidation: true, the ERC-6900 validation skips all validation for runtime calls to this selector. Any external address can call account.addAllowedRecipients([attackerAddress]), and the account forwards the call to the module without checking authorization. Since msg.sender in the module is the account itself, the attacker's address is added to the account's allowlist.

Meanwhile, removeAllowedRecipients() correctly configures validations with skipRuntimeValidation: false.

Exploit scenario

1. Account has the ColdStorageAddressBookModule execution module installed.
2. An attacker calls account.addAllowedRecipients([attackerAddress]). No validation is required. The attacker's address is added to the account's allowlist.
3. The attacker as now an allowed address. The user can be phished into sending the attacker tokens.

Recommendation
Change the manifest configuration for addAllowedRecipients() to skipRuntimeValidation: false.

[osr21]

The skipRuntimeValidation: true finding is severe. The ERC-6900 validation skip is a full authorization bypass for that function selector — any caller can invoke account.addAllowedRecipients() without any access check, and the call is forwarded by the account to the module with msg.sender appearing as the account itself.

The exploit chain:

1. Victim installs ColdStorageAddressBookModule
2. Attacker calls victimAccount.addAllowedRecipients([attackerAddr]) — no validation required, passes unconditionally
3. Attacker is now a recognized "allowed recipient" in the victim's ColdStorage context
4. Depending on timelock configuration: attacker waits out any timelock, then triggers a transfer to themselves as an allowlisted address

The only mitigating factor is a non-zero timelock in the ColdStorage config — that creates a detection window, but does not prevent the allowlist write itself. Accounts with a zero-second timelock are immediately exploitable at step 3.

The fix in PR #113 (skipRuntimeValidation: false) is correct. The asymmetry with removeAllowedRecipients (which already has validation enabled) makes this look like a copy-paste omission rather than an intentional design choice.

One additional note from an ERC-6900 / ERC-4337 implementation angle: this pattern — an execution module function with skipRuntimeValidation: true — should probably be flagged by module security tooling as a default-warn case. Any module that exposes a state-mutating function (one that writes to per-account storage) with validation skipped is asserting that the function is safe for unrestricted access, which is almost never the intended semantics. Worth considering whether future module submission guidelines should require explicit rationale for any skipRuntimeValidation: true on non-read-only selectors.