T09 · Insecure Skill Coding Practices
Error
- Location
- .tmp` temporary path is also created without exclusive-file or symbolic-link protections. This increases exposure during writes and may allow interference when an attacker already has write access to the storage directory. ### Attack Path 1. A user creates or imports an identity without configuring `BILLIONS_NETWORK_MASTER_KMS_KEY`. 2. The Skill serializes the identity's raw private key using the `plain` provider. 3. `FileStorage.writeFile` writes that data to the predictable `kms.json.tmp` path and renam ...[truncated 1157 chars]:48
- Finding
- Private Keys Are Stored in Plaintext Without Enforced Restrictive File Permissions<![CDATA[ ## Vulnerability Details **File Location**: `scripts/shared/storage/keys.js:48-61`; `scripts/shared/storage/base.js:27-32` **Vulnerability Type**: Plaintext sensitive-data storage and unsafe file permissions **Risk Level**: High ### Vulnerable Code `scripts/shared/storage/keys.js:48-61`: ```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 }, }; } ``` `scripts/shared/storage/base.js:27-32`: ```js 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 serializes the raw private key into `kms.json` with the provider set to `plain`. Encryption is therefore optional even though the stored material controls the agent's decentralized identity. The generic file-storage implementation creates the directory and temporary file without explicitly setting restrictive modes. Access is consequently determined by the process umask and pre-existing directory permissions. In an environment with permissive permissions, another local account or process may be able to read the key file. The predictable `<file>.tmp` temporary path is also created without exclusive-file or symbolic-link protections. This increases exposure during writes and may allow interference when an attacker already has write access to the storage directory. ### Attack Path 1. A user creates or imports an identity without configuring `BILLIONS_NETWORK ...[truncated 1220 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Require encrypted private-key storage and fail closed when no valid master key is configured. Do not silently fall back to plaintext. 2. If password-like master secrets are supported, derive the encryption key with a password KDF such as Argon2id or scrypt using a unique random salt and documented parameters. 3. Create `$HOME/.openclaw/billions` with mode `0700` and key files with mode `0600`, independent of the process umask. 4. Validate that the storage directory and destination are owned by the expected user and are not symbolic links. 5. Use a cryptographically random temporary filename in the same directory, create it with exclusive semantics, set mode `0600`, flush it, and then atomically rename it. 6. Check and correct the permissions of legacy `kms.json` files during migration. 7. Provide a secure migration path that encrypts existing plaintext entries after a master key is configured. 8. Avoid accepting private keys directly on the command line where feasible because command-line arguments may be exposed through shell history or process inspection. ]]>
