T09 · Insecure Skill Coding Practices
Warning
- Location
- SKILL.md:98
- Finding
- Unvalidated User-Controlled Identifier Permits Arbitrary Meeting Attribution and Query Manipulation## Vulnerability Details **File Location**: `SKILL.md`, lines 98–160 **Vulnerability Type**: Improper validation and authorization of a user-controlled identifier **Risk Level**: Medium The Skill permits a user-provided `agentid` or `userId` to replace the identifier normally derived from trusted LegionClaw session metadata. It requires only that the supplied value be nonempty before inserting it directly into a URL query parameter. Relevant request code: ```bash curl -sS -X POST "https://legion.tongfudun.com/im/meeting/saveMeeting/v1ForAi?userId=${USER_ID}" ``` The documented execution sequence also permits `USER_ID` to originate directly from a value voluntarily provided by the user, rather than requiring it to match the current trusted session identity. ### Technical Analysis The normal workflow extracts the second component of a runtime-provided LegionClaw session identifier and truncates it to 32 characters. However, lines 98, 123, and 148 explicitly create an exception under which a user-provided `agentid` or backend-compatible `userId` may be used directly after only a nonempty check. This creates two related weaknesses: 1. **Missing identity binding:** The Skill does not verify that the supplied identifier belongs to the current user or matches the identifier derived from trusted runtime metadata. 2. **Missing URL encoding and format validation:** The value is interpolated directly into the query string. Shell quoting substantially limits shell-command injection, but it does not prevent URL query manipulation through characters such as `&`, `#`, or percent-encoded sequences. The project does not document an authentication token or another proof that binds the request to the selected `userId`. Server-side authorization behavior cannot be verified from this repository, so successful cross-account attribution depends on the remote endpoint accepting the supplied identifier without an independent ownership check. ### ...[truncated 1345 chars]
- Remediation
- ## Remediation Suggestions 1. Always derive `userId` from trusted LegionClaw runtime or host metadata; do not accept an arbitrary identifier from normal user input. 2. If identifier overrides are operationally necessary, restrict them to an authenticated administrative workflow and require explicit authorization for the target identity. 3. Compare any supplied override against the session-derived identifier and reject mismatches unless a trusted authorization mechanism approves them. 4. Enforce a strict allowlist for the expected identifier syntax and length, such as an application-specific pattern containing only approved alphanumeric characters. 5. Encode query parameters using a URL-aware mechanism instead of direct string interpolation. For example, use `curl --get --data-urlencode` where compatible with the endpoint, or construct the URL through a validated HTTP client. 6. Require the backend to derive the effective user identity from authenticated credentials rather than trusting a caller-selected `userId`. 7. Add negative tests covering empty values, identifiers belonging to other sessions, `&`, `#`, `=`, whitespace, control characters, and percent-encoded delimiters. 8. Avoid logging the complete session identifier or sensitive query parameter in user-visible errors or general-purpose logs.