fix: sanitize subprocess call in _exiftool.py#1671
fix: sanitize subprocess call in _exiftool.py#1671orbisai0security wants to merge 1 commit intomicrosoft:mainfrom
Conversation
Automated security fix generated by Orbis Security AI
VANDRANKI
left a comment
VANDRANKI
left a comment
There was a problem hiding this comment.
Solid security improvement. Three things work together here:
-
shutil.which()+os.realpath()resolves the path to an absolute canonical path before it reaches subprocess. This prevents symlink-based path manipulation and catches cases where the path looks valid but does not resolve to an executable. -
--separator before-in the exiftool invocation. Without this, a filename starting with-could be interpreted as a flag. Passing content via stdin with-was already safe, but the--makes the intent explicit. -
timeout=30on the version check subprocess call, withsubprocess.TimeoutExpiredadded to the except clause. The main subprocess call - is there a timeout on that too? Large files passed via stdin could hang indefinitely if exiftool stalls. Worth adding the same timeout there.
Otherwise LGTM.
2 participants
Summary
Fix critical severity security issue in
packages/markitdown/src/markitdown/converters/_exiftool.py.Vulnerability
V-001packages/markitdown/src/markitdown/converters/_exiftool.py:41Description: The ExifTool converter passes user-controlled file paths to subprocess.run() without proper validation. While the code uses a list format (not shell=True), the local_path variable comes from user input and could contain malicious filenames. If the file path contains special characters or is manipulated through path traversal, it could lead to unintended file access or command execution through ExifTool's own command parsing.
Changes
packages/markitdown/src/markitdown/converters/_exiftool.pyVerification
Automated security fix by OrbisAI Security