aMule Forum

English => en_Bugs => Topic started by: aapo on July 12, 2004, 09:14:46 AM

Title: Bug in CUpDownClient::ProcessMuleCommentPacket(char* pachPacket, uint32 nSize)
Post by: aapo on July 12, 2004, 09:14:46 AM

Hello!

Just popped in to say, that there is a bug somewhere in function CUpDownClient:: ProcessMuleCommentPacket  in file BaseClient.cpp.  The function try'es and catches if there is a 'reqfile' pointer, but something goes wrong and amule crashes if there is a malformed MuleCommentPacket.  I've got several full crashes on this problem.  Code snippet follows:

Code: [Select]

void CUpDownClient::ProcessMuleCommentPacket(char* pachPacket, uint32 nSize)
{
        try
        {
                if (!reqfile) {
                        throw CInvalidPacket("comment packet for unknown file");
                }

                CSafeMemFile data((BYTE*)pachPacket,nSize);
                int length;
                if ( sizeof(m_iRate) != data.Read(&m_iRate,sizeof(m_iRate)) )
                        throw CInvalidPacket("short packet reading rating");
                if ( sizeof(length) != data.Read(&length,sizeof(length)) )
                        throw CInvalidPacket("short packet reading comment length");

                reqfile->SetHasRating(true);
                theApp.amuledlg->AddDebugLogLine(false,_("Rating for file '%s' received: %i"),m_pszClientFilename,m_iRate);
                if (length>50) length=50;
                if (length>0){
                        char* desc=new char[length+1];
                        memset(desc,0,length+1);
                        if ( (unsigned int)length != data.Read(desc,length) )
                                throw CInvalidPacket("short packet reading comment string");
                        theApp.amuledlg->AddDebugLogLine(false,_("Description for file '%s' received: %s"), m_pszClientFilename, desc);
                        m_strComment.Format("%s",desc);
                        reqfile->SetHasComment(true);
                        delete[] desc;
                }

        }
        catch ( CInvalidPacket e )
        {
                printf("Invalid MuleComment packet - %s\n", e.what());
                return;
        }
        if (reqfile->HasRating() || reqfile->HasComment()) theApp.amuledlg->transferwnd->downloadlistctrl->UpdateItem(reqfile);
}


Title: RE: Bug in CUpDownClient::ProcessMuleCommentPacket(char* pachPacket, uint32 nSize)
Post by: Xaignar on July 12, 2004, 01:17:12 PM

Thanks for the, uh, code-snippet. But could you post a full BackTrace as well?
Also, which version of aMule are we talking about?

Cheers,
 Xaignar

Title: Re: Bug in CUpDownClient::ProcessMuleCommentPacket(char* pachPacket, uint32 nSize)
Post by: Jacobo221 on July 12, 2004, 08:37:20 PM

He's talking about rc3, Xaignar. Look at the title ;-P

Title: Re: Bug in CUpDownClient::ProcessMuleCommentPacket(char* pachPacket, uint32 nSize)
Post by: Kry on July 12, 2004, 11:41:20 PM

Xaignar: remember the things abour wxString::Format? ;)

Title: Re: Bug in CUpDownClient::ProcessMuleCommentPacket(char* pachPacket, uint32 nSize)
Post by: Xaignar on July 12, 2004, 11:53:16 PM

hmm, I assume you're talking about it being safe to use unicoded chars on wxString::Format? It should be safe, check the code yourself, but I'm not sure if it's safe to use chars on a unicoded build and wchars on a non-unicoded build. IIRC, we should be using %hs rather than %s for that, since %hs specifes a non-unicode strings.