T09 · Insecure Skill Coding Practices
Error
- Location
- write_file.py:367
- Finding
- XLSX Append Operation Silently Overwrites Existing Spreadsheet Data<![CDATA[ ## Vulnerability Details **File Location**: `write_file.py:367-402` **Vulnerability Type**: Destructive append implementation and data integrity failure **Risk Level**: High ### Vulnerable Code ```python def append_to_xlsx(file_path, rows): """ 追加数据到 Excel 文件（简化实现：读取现有数据，合并后重新创建） Args: file_path: 文件路径 rows: 二维数组，每行数据 Returns: 写入的字节数 """ # 读取现有数据 existing_rows = [] with zipfile.ZipFile(file_path, 'r') as zf: try: shared_strings_content = zf.read('xl/sharedStrings.xml').decode('utf-8') # 提取所有字符串 import re matches = re.findall(r'<t>([^<]*)</t>', shared_strings_content) # 读取 worksheet 获取行列结构 worksheet_content = zf.read('xl/worksheets/sheet1.xml').decode('utf-8') # 简单解析：获取行数 row_matches = re.findall(r'<row r="(\d+)"', worksheet_content) if row_matches: max_row = int(max(row_matches)) # 这里简化处理，假设每行有相同列数 # 实际应该更复杂地解析 else: max_row = 0 except: pass # 合并数据并重新创建 all_rows = existing_rows + rows return create_xlsx(file_path, all_rows if all_rows else rows) ``` ### Technical Analysis The function claims to append rows to an existing XLSX workbook, but `existing_rows` is initialized as an empty list and is never populated. Although the implementation reads `xl/sharedStrings.xml` and `xl/worksheets/sheet1.xml`, the extracted strings, row references, and maximum row number are not converted into existing spreadsheet rows. Consequently, the following expression always contains only the newly supplied rows: ```python all_rows = existing_rows + rows ``` The function then calls `create_xlsx()` with the original file path. That function opens the destination in binary overwrite mode and creates a new minimal workbook. Existing cells, worksheets, formulas, styles, ...[truncated 1753 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Replace the simplified regular-expression parsing with a proven XLSX/OpenXML library such as `openpyxl`, if introducing a maintained dependency is acceptable. 2. If standard-library-only operation is required, fully parse: - Shared strings. - Inline strings. - Worksheet cell references and types. - Sparse rows and columns. - XML entities and namespaces. 3. Populate `existing_rows` before combining it with new rows. 4. Preserve all unrelated ZIP members rather than creating a new minimal workbook. 5. Do not use a bare `except:`. Catch expected exceptions explicitly and abort without modifying the original file when parsing fails. 6. Write the updated workbook to a temporary file in the destination directory. 7. Validate the completed temporary workbook, flush it to disk, and atomically replace the original only after success. 8. Retain or create a backup when destructive replacement cannot be made atomic. 9. Add regression tests verifying that: - Existing rows remain unchanged. - New rows appear after existing rows. - Styles, formulas, worksheets, and metadata are preserved. - Malformed workbooks produce an error without changing the original file. ]]>