[Pidgin] #15289: exchndl.dll issues

[Pidgin]

#15289: exchndl.dll issues
----------------------+-----------------------------------------------------
 Reporter:  ioerror   |        Owner:  datallah       
     Type:  defect    |       Status:  new            
Milestone:            |    Component:  winpidgin (gtk)
  Version:  2.10.6    |   Resolution:                 
 Keywords:  security  |  
----------------------+-----------------------------------------------------

Comment(by ioerror):

 Some other comments on other issues in the code...

 rprintf() has an internal static buffer ( TCHAR szBuff[4096] ) and it does
 not bounds check at all. In theory, this may be safe because none of the
 calls should be able to give it more than 512bytes, such as this:
 {{{
 BdfDemangleSymName(szSymName, szSymName, 512);
 ...
 rprintf( _T("  %s"), szSymName);
 }}}

 However, if we look at BfdDemangleSymName() we see that it uses lstrcpyn()
 which shouldn't be used ( http://msdn.microsoft.com/en-
 us/library/windows/desktop/ms647491(v=vs.85).aspx ) as Microsoft says
 '''Warning  Do not use. Consider using StringCchCopy instead. See
 Remarks.'''

 In one case lstrcpyn is used in in BfdDemangleSymName() exactly as it
 shouldn't be used: '''The lstrcpyn function has an undefined behavior if
 source and destination buffers overlap.'''

 In another case in BfdDemangleSymName() is takes the result of
 cplus_demangle() and puts it into res. res should be considered totally
 untrusted as a result of the lack of validation and the issues with
 LoadLibrary.

 It isn't clear to me that rprintf() in that case is actually safe in all
 cases. The bounds checking for rprintf() depends on functions that are
 declared dangerous and specifically bad to use as they are used...

-- 
Ticket URL: <http://developer.pidgin.im/ticket/15289#comment:2>
Pidgin <http://pidgin.im>
Pidgin