Re: GetFullPathName overflow - was 'Re: WFTPD "Pro" 3.0 R4 Buffer Overflow'

From: Alun Jones (alunat_private)
Date: Wed Apr 25 2001 - 14:43:48 PDT

  • Next message: Tom Yu: "Security advisory: krb5 ftpd buffer overflows"

    In the interests of full disclosure, I'm letting you know what the current
    state of play is on this problem.
    
    The following code is pulled from one of our functions:
    
    void ExpandRelativePath(char *to, const char *from)
    {
    	BOOL found=1;
    	// Relative path - let's see if GetFullPathName can expand it
    	LPTSTR lpFilePart;
    	if (from && !*from)
    	{
    		GetFullPathName("x",_MAX_PATH,to,&lpFilePart);
    		*lpFilePart=0;
    	}
    	else
    	{
    	// Scan for certain device names in the path, to disallow them.
    		if (IsDeviceNameInPath(from))
    			found=0;
    		else
    		{
    			DWORD fullen=GetFullPathName(from,_MAX_PATH,to,&lpFilePart);
    
    // directory does not exist, or is too long - abort.
    			if (fullen==0 || fullen>_MAX_PATH)
    				found=0;
    		}
    	}
    	if (!found)
    		*to=0;
    }
    
    It's the second call to GetFullPathName that eventually results in a GPF,
    within the bowels of NTDLL.DLL:RtlFreeHeap().  Since the problem appears to
    be in freeing the heap, our initial concern was to see if the heap is valid
    before calling GetFullPathName.  With this in mind, we add a line
    "HeapValidate(GetProcessHeap(),0,0);" before the second call to
    GetFullPathName().  Not only does HeapValidate indicate that indeed the
    heap is valid, but the mere process of checking the heap for validity
    avoids the bug.  In fact, merely checking if "from" is a valid pointer,
    through a call to the "IsBadReadPtr" function, is enough to prevent the bug
    from occurring.
    
    A note is made in the Microsoft Knowledge Base that HeapValidate() may
    alter the manner in which heap allocation is carried out, in the same way
    as introducing a registry setting
    "HKEY_LOCAL_MACHINE\Software\Microsoft\Windows NT\CurrentVersion\Image File
    Execution Options\wftpd.exe\DisableHeapLookAside", as a string set to the
    value "1".  Applying this registry change, however, does _not_ avoid the
    occurrence of this bug, and is not recommended.
    
    The bug shows up in both release and debug (i.e. no compiler optimisation)
    builds of WFTPD and WFTPD Pro, on Windows NT 4, at least in SP3, SP4, and
    SP6a.  It does _not_ apparently show up in Windows 2000, nor in Windows 95,
    98 or ME.  Nor does it show up if WFTPD or WFTPD Pro are run under the
    Microsoft Visual C++ 6.0 debugger, where the debug output merely displays
    "First-chance exception in Wftpd.exe (NTDLL.DLL): 0xC0000005: Access
    Violation".  [Note: the "first-chance exception" is where the debugger gets
    to handle the exception before the program.  The debugger, in this case,
    gets to ignore the exception, and pass it on to the program - which,
    apparently, is handling it when running under the debugger, but not
    handling it when running alone.]
    
    At this stage, we are mystified as to the cause of this, and as to the
    behaviour it is currently exhibiting.  Our best guess is that there is a
    subtle bug in the memory handling of Windows NT 4.0, that does not exist on
    other platforms.  We shall shortly be releasing a version of our software
    that sidesteps such behaviour, by disallowing this long parameter at an
    earlier stage.  However, this is not a fix we feel comfortable with, as we
    are unable to determine absolutely that this memory bug will not occur in
    other areas.  We welcome any input on this question.
    
    Alun.
    ~~~~
    --
    Texas Imperial Software   | Try WFTPD, the Windows FTP Server. Find us at
    1602 Harvest Moon Place   | http://www.wftpd.com or email alunat_private
    Cedar Park TX 78613-1419  | VISA/MC accepted.  NT-based sites, be sure to
    Fax/Voice +1(512)378-3246 | read details of WFTPD Pro for NT.
    



    This archive was generated by hypermail 2b30 : Wed Apr 25 2001 - 22:14:38 PDT