Avoid breakage with paths with unusual names

If file_path has any whitespace or shell metacharacters in it, then
the invocation of subprocess.call would be likely to break (or even
accidentally execute code, depending on how perverse the pathnames
are).

It's generally a good plan to avoid shell=True for subprocess.call
where you can lay out the arguments deliberately in python.  This one
looks relatively straightforward (but note, i have not tested it,
sorry!)

Note that if a name has a `"` character in it, we still fail, out of
safety reasons.

in particular, we want to avoid command injection into the sqlite
binary with particularly malicious names that look something like the
following:

```
foo.db"; .shell touch should-not-exist; .nullvalue "
```
This commit is contained in:
Daniel Kahn Gillmor
2021-07-30 23:59:12 -04:00
parent b2e210e91c
commit b2afce5c79

View File

@@ -50,13 +50,14 @@ class IOSExtraction(MVTModule):
if not shutil.which("sqlite3"):
raise DatabaseCorruptedError("Unable to recover without sqlite3 binary. Please install sqlite3!")
if '"' in file_path:
raise DatabaseCorruptedError(f"Database at path '{file_path}' is corrupted. unable to recover because it has a quotation mark (\") in its name.")
bak_path = f"{file_path}.bak"
shutil.move(file_path, bak_path)
cmd = f"sqlite3 {bak_path} '.clone {file_path}'"
ret = subprocess.call(cmd, shell=True, stdout=subprocess.PIPE,
stderr=subprocess.PIPE)
ret = subprocess.call(['sqlite3', bak_path, f'.clone "{file_path}"'],
stdout=subprocess.PIPE, stderr=subprocess.PIPE)
if ret != 0:
raise DatabaseCorruptedError("Recovery of database failed")