T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/read_sqlite.py:44
- Finding
- Unrestricted SQL Execution Against a Writable SQLite Database## Vulnerability Details **File Location**: `scripts/read_sqlite.py`, lines 44–48, 97, and 125 **Vulnerability Type**: Unrestricted state-changing SQL execution in a purported read-only utility **Risk Level**: High ### Vulnerable Code ```python def execute_query(conn, query, limit=None): """Execute a SQL query and return results""" cursor = conn.cursor() try: cursor.execute(query) ``` ```python conn = sqlite3.connect(args.database) ``` ```python elif args.query: result = execute_query(conn, args.query, args.limit) ``` ### Technical Analysis The Skill is documented as a SQLite reader that supports SELECT queries, but the implementation passes the entire user- or agent-supplied `--query` value directly to `cursor.execute()` without enforcing a read-only SQL policy. The database is opened through a normal `sqlite3.connect(args.database)` call rather than a read-only URI. Subject to filesystem permissions, this grants the connection write access. As a result, the query interface can execute state-changing statements such as `INSERT`, `UPDATE`, `DELETE`, `DROP`, and writable `PRAGMA` operations. The `--limit` option only controls how many result rows are fetched; it does not limit or neutralize SQL side effects. Although `cursor.execute()` generally rejects multiple statements in one call, this does not mitigate the issue because a single destructive statement is sufficient to alter or destroy the database. Because the documentation specifically identifies OpenClaw's `main.sqlite` memory database as an intended target, the affected data may include sessions, messages, user details, and transcript paths. ### Attack Path 1. A user or untrusted instruction causes the Skill to inspect a writable SQLite database. 2. The agent passes a state-changing statement through the `--query` argument, for example: ```sql DELETE FROM messages; ``` 3. The script opens the selected database using a writable SQLite connection. 4. `cursor.execu ...[truncated 994 chars]
- Remediation
- ## Remediation Suggestions 1. **Open the database in read-only mode** using a SQLite URI: ```python from pathlib import Path database_uri = Path(args.database).resolve().as_uri() + "?mode=ro" conn = sqlite3.connect(database_uri, uri=True) ``` 2. **Enforce read-only operations with SQLite's authorizer API.** Deny insert, update, delete, schema modification, transaction manipulation, attach/detach, and unsafe PRAGMA actions: ```python def authorize(action, arg1, arg2, database_name, trigger_name): denied = { sqlite3.SQLITE_INSERT, sqlite3.SQLITE_UPDATE, sqlite3.SQLITE_DELETE, sqlite3.SQLITE_CREATE_TABLE, sqlite3.SQLITE_DROP_TABLE, sqlite3.SQLITE_ALTER_TABLE, sqlite3.SQLITE_ATTACH, sqlite3.SQLITE_DETACH, } return sqlite3.SQLITE_DENY if action in denied else sqlite3.SQLITE_OK conn.set_authorizer(authorize) ``` 3. **Permit only read-only statements.** Do not rely solely on a textual prefix check, because comments, PRAGMAs, and complex SQL syntax can bypass simplistic validation. Use the read-only connection and authorizer as the primary controls. 4. **Reject queries that return no result-set metadata** and handle `cursor.description is None` safely. 5. **Add explicit confirmation and a separate administrative mode** if write functionality is ever intentionally introduced. It should not share the reader's default execution path. 6. **Add security tests** confirming that `INSERT`, `UPDATE`, `DELETE`, `DROP`, `ALTER`, `ATTACH`, `DETACH`, and writable PRAGMA statements are denied while legitimate `SELECT` and schema-inspection operations remain available. 7. **Update the documentation** to accurately state and guarantee that arbitrary queries are constrained to read-only behavior.