[mythtv-commits] Ticket #7886: Backend crashes when invalid network request received

[MythTV]

#7886: Backend crashes when invalid network request received
-------------------------------------------------+--------------------------
 Reporter:  Ronald Frazier <ron@…>               |       Owner:  ijr       
     Type:  defect                               |      Status:  new       
 Priority:  minor                                |   Milestone:  unknown   
Component:  MythTV - General                     |     Version:  0.22-fixes
 Severity:  medium                               |     Mlocked:  0         
-------------------------------------------------+--------------------------
 I've discovered that mythbackend can be crashed by a network request which
 includes an invalid number of parameters. My example case was using the
 perl bindings and calling new_program with a blank starttime (which itself
 makes a QUERY_RECORDING TIMESLOT request to the backend with a blank
 starttime). However, I've discovered a number of other cases where it can
 happen.

 The source of the bug is in mainserver.cpp. ProcessRequestWork reads a
 "QStringList listline" off of the socket. It then takes the first element
 (listline[0]) and splits it into "QStringList tokens". The listline and
 tokens variables are then sent to various request handlers, some of which
 access array indices without first checking the length (which is how my
 backend crash occurred).

 By the way...when listline[0] is accessed in ProcessRequestWork, it never
 verifies size() != 0 either. I'm not sure if it's possible for
 MythSocket::readStringList to create a zero length stringlist while still
 returning true. If so, this is another problem that needs to be fixed.

 I've tried following through the code, to see which functions access
 without properly checking the array size. For the tokens variable, the
 only problematic function I saw was HandleQueryRecording. I'm attatching a
 patch that fixes that.

 For the listline variable, my patch also fixes two spots in
 ProcessRequestWork that were problems...in the (command == "MESSAGE")
 block and (command == "SHUTDOWN_NOW") block. However, there were a number
 of other cases where the indices weren't verified (and a few that use
 QStringList::at(index), which I couldn't determine if that would segfault
 too) and I wasn't sure of the appropriate way to fix them (should they
 generate an error too, or proceed as if a blank line was sent instead) or
 if they needed fixing at all in the case of the at() call. So here is the
 list of (potential) problems I found that my patch does NOT address:

 ProcessRequestWork, in the (command == "BACKEND_MESSAGE") block

 HandleQueryCheckFile

 HandleSGGetFileList

 HandleSGFileQuery

 HandleGetNextFreeRecorder

 HandleRecorderQuery

 HandleSetChannelInfo

 HandleRemoteEncoder

 HandleGetRecorderFromNum

 HandleFileTransferQuery

 HandleMessage

 HandleFillProgramInfo

 HandleIsActiveBackendQuery

-- 
Ticket URL: <http://svn.mythtv.org/trac/ticket/7886>
MythTV <http://www.mythtv.org/>
MythTV