Skip to content

Commit dfa77ef

Browse files
authored
solarcam: run ffmpeg without a shell; camera-supplied file names reached it (#15)
convertFile() ran ffmpeg with shell=True on paths built from the FileName the camera returns from OPFileQuery, so a hostile camera could run commands on the downloading host whenever target_filetype_video was set. ffmpeg now gets an argv list, and generateTargetFilePath() keeps only [A-Za-z0-9_-] in the derived name; real names (2024-01-15_12-00-00) are unchanged. A failed conversion no longer deletes the downloaded recording: the source is kept, the partial output removed, and the next cycle retries the conversion without downloading again. Reported by @waseem-cve (GHSA-w2h5-ww59-7hxr).
1 parent 6ea861a commit dfa77ef

1 file changed

Lines changed: 28 additions & 11 deletions

File tree

‎solarcam.py‎

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
from pathlib import Path
44
import subprocess
55
import json
6+
import re
67
from datetime import datetime
78

89

@@ -73,6 +74,8 @@ def generateTargetFilePath(self, filename, downloadDir, extention=""):
7374
fileExtention = Path(filename).suffix
7475
filenameSplit = filename.split("/")
7576
filenameDisk = f"{filenameSplit[3]}_{filenameSplit[5][:8]}".replace(".", "-")
77+
# the segments come from the camera; keep anything else out of the path
78+
filenameDisk = re.sub(r"[^A-Za-z0-9_-]", "_", filenameDisk)
7679
targetPathClean = f"{downloadDir}/{filenameDisk}"
7780

7881
if extention != "":
@@ -81,16 +84,27 @@ def generateTargetFilePath(self, filename, downloadDir, extention=""):
8184
return f"{targetPathClean}{fileExtention}"
8285

8386
def convertFile(self, sourceFile, targetFile):
87+
# an argv list, not a shell string: the paths carry camera-supplied names
8488
if (
8589
subprocess.run(
86-
f"ffmpeg -framerate 15 -i {sourceFile} -b:v 1M -c:v libvpx-vp9 -c:a libopus {targetFile}",
90+
[
91+
"ffmpeg",
92+
"-framerate", "15",
93+
"-i", sourceFile,
94+
"-b:v", "1M",
95+
"-c:v", "libvpx-vp9",
96+
"-c:a", "libopus",
97+
targetFile,
98+
],
8799
stdout=subprocess.DEVNULL,
88100
stderr=subprocess.DEVNULL,
89-
shell=True,
90101
).returncode
91102
!= 0
92103
):
93104
self.logger.debug(f"Error converting video. Check {sourceFile}")
105+
# a partial output would be taken as converted and stop the retry
106+
Path(targetFile).unlink(missing_ok=True)
107+
return
94108

95109
self.logger.debug(f"File successfully converted: {targetFile}")
96110
Path(sourceFile).unlink()
@@ -110,10 +124,6 @@ def save_files(self, download_dir, files, blacklist=None, target_filetype=None):
110124
file["FileName"], download_dir, extention=f"{target_filetype}"
111125
)
112126

113-
if Path(f"{target_file_path}").is_file():
114-
self.logger.debug(f"File already exists: {target_file_path}")
115-
continue
116-
117127
if (
118128
target_file_path_convert
119129
and Path(f"{target_file_path_convert}").is_file()
@@ -123,6 +133,12 @@ def save_files(self, download_dir, files, blacklist=None, target_filetype=None):
123133
)
124134
continue
125135

136+
# with conversion on, a source left by a failed conversion is retried
137+
downloaded = Path(f"{target_file_path}").is_file()
138+
if downloaded and not target_file_path_convert:
139+
self.logger.debug(f"File already exists: {target_file_path}")
140+
continue
141+
126142
if blacklist:
127143
if target_file_path in blacklist:
128144
self.logger.debug(f"File is on the blacklist: {target_file_path}")
@@ -133,11 +149,12 @@ def save_files(self, download_dir, files, blacklist=None, target_filetype=None):
133149
)
134150
continue
135151

136-
self.logger.debug(f"Downloading {target_file_path}...")
137-
self.cam.download_file(
138-
file["BeginTime"], file["EndTime"], file["FileName"], target_file_path
139-
)
140-
self.logger.debug(f"Finished downloading {target_file_path}...")
152+
if not downloaded:
153+
self.logger.debug(f"Downloading {target_file_path}...")
154+
self.cam.download_file(
155+
file["BeginTime"], file["EndTime"], file["FileName"], target_file_path
156+
)
157+
self.logger.debug(f"Finished downloading {target_file_path}...")
141158

142159
if target_file_path_convert:
143160
self.logger.debug(f"Converting {target_file_path_convert}...")

0 commit comments

Comments
 (0)