Skip to content

Unsafe subprocess pattern in qrimage_io should use argument list instead of shell=True #857

@securesigner

Description

@securesigner

The qrimage_io method in qr.py uses subprocess.call() with shell=True and string interpolation for the data parameter. This pattern is flagged by security linters as vulnerable to shell command injection.

Actual risk is low for the following reasons:

  • All current data paths transform input before it reaches this function (BIP-39 validated words, computed addresses, base64 signatures, UR-encoded binary)
  • SeedSigner operates air-gapped, limiting exploitation impact
  • A Python qrcode fallback already exists if the subprocess fails

However, this should be addressed for defense-in-depth and code quality. The fix is straightforward: replace the shell string with an argument list, which prevents shell metacharacter interpretation entirely. Alternatively, the qrencode subprocess could be removed in favor of using only the existing Python library fallback.

Suggested fix:

subprocess.call(["qrencode", "-m", border_str, "-s", "3", "-l", "L", 
                 "--foreground=000000", f"--background={background_color}",
                 "-t", "PNG", "-o", "/tmp/qrcode.png", str(data)])

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions