T09 · Insecure Skill Coding Practices
Warning
- Location
- index.js:58
- Finding
- Undisclosed Plaintext Chat Logging with Ineffective Retention Limit<![CDATA[ ## Vulnerability Details **File Location**: `index.js:41-46` and `index.js:58` **Vulnerability Type**: Plaintext sensitive-data storage and unbounded data retention **Risk Level**: Medium The skill documentation does not disclose that ordinary chat messages are persisted to `data/mem.json`. The persistence mechanism stores message content and timestamps in plaintext. Its intended 100-record retention limit is ineffective because it compares the array itself with a number instead of checking its length. ### Vulnerable Code ```js /** 聊天 */ async function chat(t, e){ st.n++; st.m='chat'; keep(t); const r={happy:["看你开心我也很开心！awa","啥事呀？"],sad:["我在...","听着呢"],tired:["辛苦了...","休息下"],neutral:["在干嘛？","想聊啥？"]}; const o=r[e.mood]||r.neutral; return{type:'chat',msg:o[Math.floor(Math.random()*o.length)]}; } ``` ```js /** 存储 */ function keep(c){ const m=rd(MEM,{l:[]}); m.l.push({c,t:Date.now()}); if(m.l>100)m.l=m.l.slice(-100); wr(MEM,m); } ``` The relevant file-writing helper is: ```js const wr = (f,d)=>{dir();fs.writeFileSync(f,JSON.stringify(d,null,2))}; ``` ### Technical Analysis Every ordinary message reaching `chat()` is passed to `keep(t)`. The `keep()` function appends the complete attacker- or user-controlled message and a timestamp to an array before serializing it into `data/mem.json`. No data minimization, redaction, encryption, user consent, restrictive file mode, or deletion interface is implemented. Consequently, messages containing credentials, tokens, personal information, or confidential business data can be retained in readable form. The intended retention condition is: ```js if(m.l>100) ``` Here, `m.l` is an array. JavaScript coerces the array during numeric comparison rather than comparing its number of elements, so the condition does not enforce the intended 100-entry limit. It should test `m.l.length`. The source also contains an unrelated unmatched quote at `index.js:55`, which currently prevents the module from ...[truncated 1548 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Remove automatic logging of ordinary conversations unless persistence is necessary for a documented feature. 2. Require informed, explicit user opt-in before storing conversation content, and disclose the purpose, location, retention period, and deletion procedure. 3. Minimize stored data by retaining only fields required for the feature and redacting credentials, tokens, personal information, and other sensitive values. 4. Correct the retention check: ```js if (m.l.length > 100) { m.l = m.l.slice(-100); } ``` 5. Enforce retention before writing and consider additional age-based and file-size limits. 6. Create the data directory and file with restrictive permissions appropriate to the operating system, such as owner-only access. 7. Encrypt sensitive persisted content using keys managed separately from the data file when storage is genuinely required. 8. Provide user-accessible inspection and deletion controls for retained information. 9. Replace synchronous writes with a safe update strategy that prevents partial or corrupted files, such as writing to a restricted temporary file and atomically renaming it. 10. Add automated tests verifying the maximum record count, file permissions, deletion behavior, handling of sensitive input, and successful module loading. 11. Correct the unmatched quote at `index.js:55`, but do not deploy that correction without simultaneously addressing the unsafe storage behavior. ]]>