Potential bug in sending files (+proposed fix)

[jmarshallnz]

There is a bug I've just fixed in the XBFileZilla
version we use in XBoxMediaCenter that appears to still
have the potential to be there in the latest CVS code
of FileZilla.

XBFileZilla that we use is based off FileZilla version
0.8.3 or thereabouts, so is reasonably old, however the
code in question has not changed all that much.

The interesting thing is that the bug itself only crops
up on certain filesizes, and only on certain systems
(network setups). For instance, I couldn't get it to
reproduce on my system at all, yet the same executeable
screwed up on several other systems.

The bug is in TransferSocket.cpp, around line 518.
There is a block of code that checks whether the number
of bytes read from the ReadFile() call is 0, and if
so, closes the file and stops the transfer (calling
Close() etc.)

The problem that occurs is that the buffer is filled at
the previous call to CTransferSocket::OnSend(), yet is
not fully emptied on that call (due to network traffic
complications or whatever). Thus, the call to
ReadFile() returns no data read, and so the code block
closes the transfer, effectively terminating the
transfer prematurely. No error is reported, so the
user is unaware of the corrupted transfer, unless a CRC
check etc. is issued later.

The fix, ofcourse, is to check first for an empty
buffer before calling Close().

In our local version of the code, I've changed:

if (!numread)
{

CloseHandle(m_hFile);
m_hFile = INVALID_HANDLE_VALUE;
m_status = 0;

m_pOwner->m_pOwner->PostThreadMessage(WM_FILEZILLA_THREADMSG,
FTM_TRANSFERMSG, m_pOwner->m_userid);

Close();
return;

}

to:

if (!numread)
{

CloseHandle(m_hFile);
m_hFile = INVALID_HANDLE_VALUE;

if (!m_nBufferPos)
{ only close if we've actually finished!

m_status=0;

m_pOwner->m_pOwner->PostThreadMessage(WM_FILEZILLA_THREADMSG,
FTM_TRANSFERMSG, m_pOwner->m_userid);

Close();
return;

}

}

Let me know if I can be of more assistance, or if you
have any idea of why it cropped up in the first place ;)

Cheers,
Jonathan

[Tim Kosse]

Thanks for spotting this serious bug.
I'm using FileZilla Server every day and did fully trust its
ability to correctly send files, so I'm quite suprised.
Looking at the code, this bug is obious, I've no clue why it
did slip through. Guess I'm getting "betriebsblind" as I
would say in German. (I'm not sure there's an equivalent
term in English, at least I don't know it)

I plan to publish a fixed version until Thursday. this week.

[Tim Kosse]

fixed in cvs