[LOW] jfiles.copy_file() builds a shell command from unescaped paths (os.system + quote wrapping) #43

Summary

jfiles.copy_file() builds a shell command line from its path arguments:

additional/python/jfiles.py:181

def copy_file(source_path, destination_path):
    os.system("cp '" + source_path + "' '" + destination_path + "'")

The single-quote wrapping is defeated by any ' in a path (cp '''; rm -rf /; echo '''' ... — the quote closes the quoted region and the rest becomes shell syntax), and a newline or $(...) is passed to /bin/sh verbatim. os.system runs the string through the shell, so no character except a closing quote is safe.

jfiles.py is the shared file-helper library imported by the privileged root scripts (run_multiple_commands.py, setup_automatic_snapshots.py, check_security.py, …), so this helper executes with root privileges in the contexts where it is used. Today the only caller passes constant paths — additional/python/setup_automatic_snapshots.py:9 (copy_file("/etc/timeshift/default.json", "/etc/timeshift/timeshift.json")) — so the flaw is latent, not currently exploitable. But it is a live os.system in a root-context helper, one new caller away from becoming a root injection primitive (the project has already removed exactly this pattern from the Dart command queue, see issue #42).

Impact

Latent OS command injection in a root-context helper. Any future call with a user- or disk-derived path (snapshot names, mount paths, restored files) becomes a root command-execution bug. Even today it silently misbehaves on paths containing quotes.

Complexity

• Complexity: 1 (Trivial)

Evidence

• additional/python/jfiles.py:180-181

  def copy_file(source_path, destination_path):
      os.system("cp '" + source_path + "' '" + destination_path + "'")

• Call site: additional/python/setup_automatic_snapshots.py:9 — root context (ensure_root_privileges() on line 7).
• Reproduce the injection pattern directly:

  python3 -c "import os; p=\"x'; touch /tmp/pwned; echo '\"; os.system(\"cp '\" + p + \"' /tmp/dst'\")" && ls -la /tmp/pwned

Recommended Fix

Replace the shell invocation with the stdlib copy, which takes paths as data and never spawns a shell:

import shutil

def copy_file(source_path, destination_path):
    shutil.copy2(source_path, destination_path)

shutil.copy2 additionally preserves metadata, which matters for the timeshift config copy. Add a regression test in additional/python/tests/ asserting copy_file copies a path containing a single quote and a space.

References

• CWE-78: OS Command Injection — https://cwe.mitre.org/data/definitions/78.html
• Python shutil.copy2 — https://docs.python.org/3/library/shutil.html#shutil.copy2
• OWASP OS Command Injection Defense Cheat Sheet — https://cheatsheetseries.owasp.org/cheatsheets/OS_Command_Injection_Defense_Cheat_Sheet.html

Validation

attacker_source: n/a (latent — no current caller passes untrusted data; source_path/destination_path would be the injection vector once any file/disk-derived path reaches copy_file)
missing_guard: os.system executes the concatenated string through /bin/sh; the naive single-quote wrapping is not shell escaping and any apostrophe in a path terminates the quoted region
sink_effect: shell code embedded in a path argument executes with the privileges of the caller — root in every current import context (setup_automatic_snapshots.py runs after ensure_root_privileges)
preconditions: a future caller passes a non-constant path (e.g. a Timeshift snapshot name or a user-selected file) into copy_file; today the only call site uses two constant paths
proof_anchors: additional/python/jfiles.py:181 — "os.system(\"cp '\" + source_path + \"' '\" + destination_path + \"'\")"; additional/python/setup_automatic_snapshots.py:9 — "jfiles.copy_file(\"/etc/timeshift/default.json\", \"/etc/timeshift/timeshift.json\")"
suggested_validation: grep -n "os.system" additional/python/jfiles.py