access_control initialize checks persistent().has() as re-init guard but writes admin to instance() storage — re-initialization bypass

[Mac-5]

Description

contracts/access_control/src/lib.rs lines 46–57 contain a storage-tier mismatch identical in structure to the treasury bug (issue #182):

pub fn initialize(env: Env, admin: Address) -> Result<(), KoraError> {
    // Guard reads from PERSISTENT storage
    if env.storage().persistent().has(&DataKey::Admin) {
        return Err(KoraError::AlreadyInitialized);
    }
    // But the write goes to INSTANCE storage
    env.storage().instance().set(&DataKey::Admin, &admin);
    env.storage().instance().set(&DataKey::Paused, &false);
    // ...
}

Because Soroban's persistent() and instance() are separate namespaces, the guard persistent().has(&DataKey::Admin) will never be true after initialization (since admin was written to instance(), not persistent()). This means:

1. The re-initialization guard is permanently bypassed — initialize can be called multiple times.
2. A second initialize call can overwrite the admin address and reset the Paused flag to false, even on a live protocol.
3. An attacker who can call initialize on a deployed contract can silently take admin rights.

Fix

Use a consistent storage tier throughout. Since require_admin reads admin from instance(), use instance() for the guard too:

if env.storage().instance().has(&DataKey::Admin) {
    return Err(KoraError::AlreadyInitialized);
}

Or, if persistent storage is preferred for archival safety, write admin to persistent() and update require_admin to read from there consistently.

Acceptance Criteria

• The re-init guard uses the same storage tier as the admin write.
• Calling initialize twice returns KoraError::AlreadyInitialized on the second call.
• A test asserts: initialize → initialize again → error.
• require_admin reads from the same tier the admin was written to.

Complexity: Low (75 points)