T09 · Insecure Skill Coding Practices
Error
- Location
- sth_video_generator.py:84
- Finding
- SQL Injection Through Unparameterized Database Queries<![CDATA[ ## Vulnerability Details **File Location**: `sth_video_generator.py:84`, `sth_video_generator.py:99`, `sth_video_generator.py:399-404`; equivalent patterns also occur in `sth_video_generator_parallel.py:167,190,436-438` and multiple scripts under `scripts/` **Vulnerability Type**: SQL injection **Risk Level**: High ### Vulnerable Code ```python def get_template_data(template_id: str) -> Optional[Dict[str, str]]: """Fetch template data from song_templates table.""" query = f"SELECT image_url, generate_video_prompt, song_type_id FROM song_templates WHERE id = '{template_id}';" result = run_psql(query) ``` ```python def get_audio_mix_url(song_type_id: str) -> Optional[str]: """Fetch audio mix URL from song_types table.""" query = f"SELECT amix_url FROM song_types WHERE id = '{song_type_id}';" result = run_psql(query) return result if result else None ``` ```python def update_template_urls(template_id: str, video_url: str) -> bool: """Update the song_templates table with video URLs.""" query = f"""UPDATE song_templates SET video_url = '{video_url}', video_url_seedream_v4 = '{video_url}' WHERE id = '{template_id}';""" result = run_psql(query) return result is not None ``` Additional confirmed locations include: - `sth_video_generator_parallel.py:167` - `sth_video_generator_parallel.py:190` - `sth_video_generator_parallel.py:436-438` - `scripts/batch_processor.py:53,63` - `scripts/check_csv_over_12s.py:62,71,80` - `scripts/filter_over_12s.py:62,71` - `scripts/filter_templates.py:60,69` - `scripts/rerun_over_12s.py:62,87,96,105,114,137` - `scripts/resume_filtered_12s.py:54,59` ### Technical Analysis The code builds SQL statements by directly interpolating values into quoted SQL literals. The `template_id` value comes from a user-supplied CSV file. Other interpolated values come from database records or the external MCP service. The resulting query is pas ...[truncated 2127 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Replace `psql -c` string construction with a PostgreSQL library such as `psycopg` or `psycopg2`. 2. Parameterize every value, including identifiers, song-type IDs, and URLs: ```python with conn.cursor() as cursor: cursor.execute( """ SELECT image_url, generate_video_prompt, song_type_id FROM song_templates WHERE id = %s """, (template_id,) ) ``` ```python with conn.cursor() as cursor: cursor.execute( """ UPDATE song_templates SET video_url = %s, video_url_seedream_v4 = %s WHERE id = %s """, (video_url, video_url, template_id) ) ``` 3. Apply the parameterized approach consistently to all affected maintenance scripts. `scripts/sync_template_data.py:46-50` already demonstrates an appropriate parameterized query pattern. 4. Validate IDs before database use. If IDs are UUIDs, parse them with `uuid.UUID`; otherwise, enforce a strict documented character set and length. 5. Grant the runtime database account only required `SELECT` and narrowly scoped `UPDATE` permissions. 6. Use explicit transactions and roll back on any failure. 7. Add regression tests containing quotes, comment markers, statement separators, and malformed identifiers. 8. Avoid logging complete SQL statements containing externally derived values. ]]>