aMule Forum

Please login or register.

Login with username, password and session length
Advanced search  

News:

We're back! (IN POG FORM)

Author Topic: Bug in CUpDownClient::ProcessMuleCommentPacket(char* pachPacket, uint32 nSize)  (Read 3604 times)

aapo

  • Newbie
  • Karma: 0
  • Offline Offline
  • Posts: 1

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);
}
« Last Edit: July 12, 2004, 09:19:10 AM by aapo »
Logged

Xaignar

  • Admin and Code Junky
  • Hero Member
  • *****
  • Karma: 19
  • Offline Offline
  • Posts: 1103

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
Logged

Jacobo221

  • Hero Member
  • *****
  • Karma: 3
  • Offline Offline
  • Posts: 2712

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

Kry

  • Ex-developer
  • Retired admin
  • Hero Member
  • *****
  • Karma: -665
  • Offline Offline
  • Posts: 5795

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

Xaignar

  • Admin and Code Junky
  • Hero Member
  • *****
  • Karma: 19
  • Offline Offline
  • Posts: 1103

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.
Logged