T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/shared/storage/keys.js:47
- Finding
- Private keys are stored in plaintext by default without enforced file permissions<![CDATA[ ## Vulnerability Details **File Location**: `scripts/shared/storage/keys.js:47-63`; `scripts/shared/storage/base.js:9-12, 27-31` **Duplicated Location**: `skills/agent-desapetc-123/scripts/shared/storage/keys.js:47-63`; `skills/agent-desapetc-123/scripts/shared/storage/base.js:9-12, 27-31` **Vulnerability Type**: Plaintext sensitive-data storage and unsafe file permissions **Risk Level**: High ### Vulnerable Code ```js _encodeEntry({ alias, privateKeyHex, createdAt }) { const masterKey = getMasterKey(); if (masterKey) { return { version: 1, provider: "encrypted", data: { alias, key: encryptKey(privateKeyHex, masterKey), createdAt }, }; } return { version: 1, provider: "plain", data: { alias, key: privateKeyHex, createdAt }, }; } ``` The generic storage implementation writes the resulting data without explicit restrictive permissions: ```js async ensureDirectory() { const dir = path.dirname(this.filePath); await fs.mkdir(dir, { recursive: true }); } async writeFile(data) { await this.ensureDirectory(); const json = JSON.stringify(data, null, 2); const tempPath = `${this.filePath}.tmp`; await fs.writeFile(tempPath, json, "utf-8"); await fs.rename(tempPath, this.filePath); } ``` ### Technical Analysis When `BILLIONS_NETWORK_MASTER_KMS_KEY` is absent or rejected, `_encodeEntry` deliberately stores the raw private key in `kms.json`. Encryption is therefore optional rather than a secure default. The storage layer also creates the directory and temporary file without specifying modes such as `0700` for the directory and `0600` for files. The effective permissions consequently depend on the process umask and pre-existing directory permissions. The temporary file may contain the complete plaintext key before it is renamed. AES-256-GCM is used when a master key is configured, but that does not mitigate deployments that omit the optional environment variable. The documentation explicitly ack ...[truncated 1420 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Remove the silent plaintext fallback. Refuse to create or import an identity unless a secure key-encryption mechanism is configured. 2. Prefer an operating-system keychain, hardware-backed keystore, TPM, HSM, or managed secret store instead of a password-derived file-encryption key. 3. Create `$HOME/.openclaw/billions` with mode `0700` and verify its ownership before reading or writing sensitive data. 4. Create key files and temporary files with mode `0600`. Use exclusive creation flags to avoid writing through attacker-prepared files. 5. Reject symbolic links and verify the owner and mode of existing storage files. 6. If a temporary file remains necessary, use a securely generated unique name in the same protected directory, flush it before replacement, and remove it on failure. 7. Provide a migration utility that encrypts existing plaintext entries and securely removes old plaintext artifacts. 8. Warn users that keys previously stored in plaintext must be considered exposed if local permissions or backups were not adequately protected. ]]>