Opened 12 years ago

Closed 2 years ago

#10680 closed Developer Task (Trac EOL)

MythSystem doesn't split command line strings internally

Reported by: github@… Owned by: Raymond Wagner
Priority: minor Milestone: needs_triage
Component: MythTV - MythSystem Version: Master Head
Severity: medium Keywords:
Cc: Ticket locked: no

Description (last modified by Raymond Wagner)

The rewritten MythSystem? class allows arguments to be passed in as list, which in turn allows MythSystem? to run external applications directly using an execv() system call. If the old style of using myth_system() is called instead, MythSystem? leaves processing of the command string up to the local system shell, leaving the possibility of misinterpretation. Add an internal mechanism to handle splitting up those command strings into argument lists such that that can be run directly as well, bypassing any potential issues caused by improper shell escaping.

Change History (6)

comment:1 Changed 12 years ago by Raymond Wagner

Component: MythTV - GeneralMythTV - MythSystem
Description: modified (diff)
Owner: set to Raymond Wagner
Status: newaccepted
Summary: MythSystem doesn't escape shell argumentsMythSystem doesn't split command line strings internally
Type: Bug Report - GeneralDeveloper Task

comment:2 Changed 12 years ago by github@…

I filed this ticket to cover a specific problem, and the rewritten ticket description doesn't seem to cover it. There are lots of places in the code that pass arguments to MythSystem as a QStringList, without the kMSNoRunShell flag, and that use their own broken escaping mechanisms instead. For example, a picture with quotes and pipes in it would inadvertantly trigger an external executable, (see 50f91450b3136cc5d0e832946d6b161ff640fcfb), even though the arguments are in a QStringList. I saw about 63 places in the code that suffer from insecure shell escaping.

To be perfectly clear: this is a security threat. Some Myth installations are configured to read pictures from an inserted thumb drive. If a drive contains malicious filenames, with single quotes and pipes in them, then the owner of the drive could take over the Myth system.

How do we address this situation? So far I've filed two tickets on it and they have both been closed or repurposed. Thanks.

comment:3 Changed 12 years ago by Raymond Wagner

Yes. The rewritten ticket description completely covers your problem. The problem you suggest is one of lack of shell escaping, allowing bad characters to be passed to the shell. If you remove the shell, you remove the problem. Not mentioned was a PATH scanner to find relative applications, but such would only take a few minutes to write. Also not mentioned is the need to enable and test shell-less operation on all those places that use external calls, but that was assumed to be implicitly understood.

To your comment about security issues, MythTV is currently rife with them, and the possibility of someone inserting a USB stick with an image file designed to induce a filename injection attack is the least of your concerns. Anyone with network access to either the protocol or web servers has full control over the machine. MythTV is designed for home use, where it sits on a private network and shouldn't have to fear its users. While it would be nice to close up these holes, that desire is tempered by the desire to make MythTV easier to configure and maintain for potential less technically minded users. You can only go so far in improving one before you begin to hurt the other.

Based off your sudden persistence on this issue that has been around since before I even started using MythTV, my best guess is that you're trying to use MythTV to run some form of public kiosk that you don't want compromised. You need to be aware that there are be tons of other holes that will need plugging as well.

comment:4 Changed 12 years ago by github@…

I see that I am trying to solve the wrong problem here. When I noticed that some image files weren't working in MythGallery, I created a patch that would fix it. You weren't interested in my fix, because I didn't take advantage of kMSNoRunShell. Then I offered to patch the MythSystem clients so that they used kMSNoRunShell instead of a shell, but you weren't interested in that either. Fine. So these images will continue to be broken. I'll just have to trust that you have a plan to get them working again, and that it will be easier for you to do it working alone.

comment:5 Changed 4 years ago by Stuart Auchterlonie

Milestone: unknownneeds_triage

comment:6 Changed 2 years ago by Stuart Auchterlonie

Resolution: Trac EOL
Status: acceptedclosed

We have moved all bug tracking to github [1]

If you continue to have this issue, please open a new issue at github, referencing this ticket.

[1] - https://github.com/MythTV/mythtv/issues

Note: See TracTickets for help on using tickets.