aMule Forum
English => en_Bugs => Topic started by: Excalibre on January 02, 2008, 07:41:02 PM
-
I'm running aMule 2.2.0 on Ubuntu Gutsy-64 and it crashes whenever I receive a message, which means every couple hours when I get a spam message the thing goes down. Every log file ends with me receiving a message. I'm running one of the distro-specific packages and I'm now trying filtering all incoming messages (don't know if it's working yet) but I wanted to know if this is a behavior anyone else has seen.
-
Yes, I've seen that and tried to backtrace it. Seems like the wxString carrying the message gets so broken that aMule crashes on access of it.
At a quick glance I would suggest to change ClientTCPSocket.cpp/ProcessPacket
wxString message = message_file.ReadString(m_client->GetUnicodeSupport());
to
wxString message = message_file.ReadString(m_client->GetUnicodeSupport(), 2, true);
to activate SafeRead mode of CMemFile/CFileDataIO::Readstring .
As it is now, a malicious or buggy client can send a message with a longer length preceeding the content, and Readstring reads happily beyond its buffer.
I haven't tested it though. I had the crashes one evening, and never since, so I'm convinced they are provoked by certain malformed packets from outside.
Cheers, Stu
-
Yep, I've also seen this, but since my amule is usually compiled with no debug, I could never post a useful backtrace. Thanks for the suggestion, sturedman.
Regards.
-
sturedman,
Your suggestion was accepted, your modification has been committed.
Thanks a lot!
-
Actually, this shouldn't fix it, and moreover, is probably the wrong approch anyway:
The reason why it shouldn't fix it, is that we already handle any EOF exceptions that might result from trying to read a too long string (it's handled in CClientTCPSocket::PacketReceived where ProcessPacket is called from). Hence, no crash should result, suggesting that the problem lies elsewhere. CMemFile::doRead itself does not permit readings beyond its buffer, happily or not, and if it does happen, then it's CMemFile that should be fixed.
And as we now have no idea if the client sent a (possibly) corrupted message packet, we now cannot do the appropriate thing. In fact, given that the package is corrupted, we might even display an invalid string (making me wonder why we have "SafeRead" in the first place).
So I would vote for reverting that commit. If that was the cause, then amule should display an appropriate message on the console ("Terminated after throwing an instance of ..."), due to the unhandled exception, and I would ask Excalibre to try running aMule from a console and checking for any such error-messages.
sturedman, if you have a backtrace, please post it. ;)
-
Sorry, don't have it anymore. :(
The crash was definitely not in the reception of the message packet. It was on access of the wxString carrying the message when amuled tried to send it to the remotegui (then I was working on that feature). Seems the wxString structure itself got corrupted somehow, strange as it may sound.
-
sturedman, if you have a backtrace, please post it. ;)
I posted a backtrace about this crash some time ago here: http://forum.amule.org/index.php?topic=13914.0
EDIT: grammar: post->posted
-
I post a backtrace about this crash some time ago here: http://forum.amule.org/index.php?topic=13914.0
Am I understanding you correctly that the crash can be avoided either by filtering all messages, or by disabling GeoIP?
-
I post a backtrace about this crash some time ago here: http://forum.amule.org/index.php?topic=13914.0
Am I understanding you correctly that the crash can be avoided either by filtering all messages, or by disabling GeoIP?
Yes, that is what happens inmy machine.
-
I'm afraid I mixed two things up. :-[
The crash I was talking about all the time occured on arrival of a certain file comment.
The subject here is a message however. So ignore my comments please.
-
Xaignar, ok I have reverted the patch.