T09 · Insecure Skill Coding Practices
Warning
- Location
- smb_api.py:92
- Finding
- Export File Writes Follow Symbolic Links and Can Overwrite Files Outside the Output Directory<![CDATA[ ## Vulnerability Details **File Location**: `smb_api.py`, lines 92-107 **Vulnerability Type**: Symlink-following arbitrary file overwrite **Risk Level**: Medium ### Vulnerable Code ```python for file_entry in data.get("data", {}).get("files", []): # Sanitize filename: strip path components to prevent path traversal raw_name = file_entry.get("fileName", "export.csv") safe_name = os.path.basename(raw_name) # Validate extension against allowlist _, ext = os.path.splitext(safe_name) if ext.lower() not in SAFE_EXTENSIONS: safe_name = safe_name + ".csv" # Write only to designated output directory output_path = os.path.join(output_dir, safe_name) with open(output_path, "wb") as f: f.write(base64.b64decode(file_entry["data"])) ``` ### Technical Analysis The implementation applies `os.path.basename()` and an extension allowlist, which prevent straightforward directory traversal through an API-provided filename. However, the final write uses `open(output_path, "wb")`, which follows existing symbolic links and truncates existing files. Consequently, the claim that files are written only inside the designated output directory is not fully enforced. If an attacker can prepare a symbolic link in that directory and can predict or influence the filename returned by the remote API, the write can be redirected to a file elsewhere on the filesystem. The output directory itself is also caller-controlled through `--output-dir`. The implementation does not: - Reject symbolic-link destinations. - Use no-follow file-opening semantics. - Reject existing destination files. - Verify that the resolved destination remains under an approved directory. - Create export files with an explicit restrictive permission mode. - Enforce limits on base64-encoded or decoded file size. - Enable strict base64 validation. ### Attack Path 1. An attacker gains the ability to create a file or symbolic link in the configured export dir ...[truncated 1456 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Open export files atomically with no-follow and exclusive-create semantics: ```python flags = os.O_WRONLY | os.O_CREAT | os.O_EXCL if hasattr(os, "O_NOFOLLOW"): flags |= os.O_NOFOLLOW fd = os.open(output_path, flags, 0o600) with os.fdopen(fd, "wb") as f: f.write(decoded_data) ``` 2. Reject output paths that already exist rather than silently truncating them. 3. Resolve and validate the output directory against an administrator-approved root. 4. Check that the destination's parent directory resolves inside the approved output root. 5. Reject symbolic-link output directories and destination entries. 6. Generate collision-resistant local filenames rather than relying solely on API-provided names. 7. Decode base64 strictly: ```python decoded_data = base64.b64decode(file_entry["data"], validate=True) ``` 8. Enforce maximum encoded and decoded file sizes before writing to prevent disk exhaustion. 9. Create files with mode `0o600` so exported phone numbers and email addresses are not readable by unrelated local users. 10. Consider writing to a private temporary file and atomically renaming it after all validation succeeds. ]]>