T09 · Insecure Skill Coding Practices
Warning
- Location
- references/storage-patterns.md:142
- Finding
- Unauthenticated Public Function Permits Forged Audit-Log Records## Vulnerability Details **File Location**: `references/storage-patterns.md`, lines 142-155 **Vulnerability Type**: Missing authentication and caller impersonation **Risk Level**: Medium ```move public entry fun log_transaction( action: String, executor: address, amount: u64 ) acquires AuditLog { let log = borrow_global_mut<AuditLog>(@my_addr); smart_vector::push_back(&mut log.transactions, TxRecord { timestamp: timestamp::now_seconds(), action, executor, amount, }); } ``` ### Technical Analysis The reusable example exposes `log_transaction` as a public entry function without accepting a signer or performing an authorization check. Consequently, any transaction sender can invoke it. The `executor` field is supplied directly by the caller rather than being derived from an authenticated signer. An attacker can therefore attribute an arbitrary action and amount to any address, including an administrator or another user. The record is then appended to the authoritative on-chain `AuditLog`. The function also lacks validation or size limits for `action` and does not constrain `amount`. Repeated calls can grow the stored log and impose avoidable storage costs. ### Attack Path 1. The victim deploys a contract based on this documented storage pattern. 2. An attacker invokes the public `log_transaction` entry function. 3. The attacker supplies a privileged or victim address as `executor`. 4. The attacker supplies arbitrary `action` and `amount` values. 5. The function appends the forged record without checking the transaction sender. 6. Indexers, monitoring systems, accounting logic, or users may treat the forged record as an authentic action by the impersonated address. 7. The attacker can repeat the operation to pollute the log and increase storage consumption. ### Impact Assessment This issue does not directly grant account or admin ...[truncated 544 chars]
- Remediation
- ## Remediation Suggestions - Add a `caller: &signer` parameter to the entry function. - Derive the executor from `signer::address_of(caller)` instead of accepting an arbitrary executor address. - If only trusted code may create records, compare the caller against a stored administrator or operator role before modifying the log. - Validate `action` with non-empty and maximum-length constraints. - Validate `amount` according to the application’s permitted range. - Emit authenticated events instead of maintaining an indefinitely growing on-chain history when records are only needed for off-chain querying. - If on-chain storage is required, implement retention or bounded-capacity controls. A safer interface would follow this structure: ```move public entry fun log_transaction( caller: &signer, action: String, amount: u64 ) acquires AuditLog { let executor = signer::address_of(caller); // Validate caller authority, action length, and amount here. // Append the authenticated record or emit an event. } ```