Sending a lot of empty lines to main chat makes DC++ to hang

Bug #1681153 reported by Sopor
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
DC++
Fix Released
Critical
Unassigned

Bug Description

If i send 15000 empty lines into main chat from another client, DC++ will hang (sometimes i need to do it twice). When i then restart DC++ i can see a message '65766: Buffer overflow' from the chat history. So if the hubsoft have no line limit you can hang a lot of DC++ clients in the hub.

I'm running DC++ 0.865.

eMTee (realprogger)
information type: Public → Private Security
Revision history for this message
eMTee (realprogger) wrote :

Can you please be more specific about what is exactly the 'empty line' you sending? Nullstring or a CR or/and LF without text, etc... Or maybe just iclude a protocol talk log?
Does this work also for private messages?

Revision history for this message
eMTee (realprogger) wrote :

I did a test with the help of the reporter and it concluded that the following mainchat message is successfully hanging DC++ 0.865 x64 on Win7 on their test Luadch v2.18 hub with the Max protocol command length DC++ setting set to the default value of 512KiB:

BMSG J7XB Main\schat\scleaning\sstarted!<Insert 15000 \n here>Main\schat\shave\sbeen\scleaned!\n

Sometimes the message needs to be sent twice. The freeze starts a few seconds after the receival. The DC++ chat log file (if enabled) is updated correctly with the received message(s) before the freeze.

The '65766: Buffer overflow' message the reporter seen was because a long MOTD and chat log script output in the test hub that exceeded 512k in length (a more user friendly message maybe?).
Disabling these scripts on the hub made no difference; without them the buffer overflow message went away but the freeze and 100% CPU happens regardless after receiving the above mainchat message once or twice.

Changed in dcplusplus:
status: New → Confirmed
importance: Undecided → Critical
summary: - sending a lot of empty lines to main will make dc++ hang
+ Sending a lot of empty lines to main chat makes DC++ to hang
Revision history for this message
poy (poy) wrote :
Revision history for this message
poy (poy) wrote :

this is a limitation in the Rich Edit control we use to display chat messages.

rev 62243417be3658a8aeaf25d6b6a6a0432df2b4bf adds a little test one can easily tweak to reproduce.

the most straight-forward resolution is to cut these messages when it is known they are hitting on Rich Edit limits.

Revision history for this message
poy (poy) wrote :
Revision history for this message
poy (poy) wrote :
Revision history for this message
poy (poy) wrote :

maybe I should have clarified in my previous message - I was hoping others would look into this now that a test case is there; rebuilds in a breeze with:
scons build/[...]/dwt/test

just pushed a few tweaks; interestingly, adding 2 chars or more per line fixes the issue here... (but 1 is not enough...)

so a workaround for the moment may be to send " \n" (3 spaces and \n) instead of just "\n" X times if you really want to "clear" chats...

I also tried writing to a file instead of appending to the text box (it worked) to confirm this is not related to our RTF conversion tools.

I hope this is enough info for one of our contributors to come up with a proper fix. :)

Revision history for this message
poy (poy) wrote :

hoho, this is actually caused by our code!

the offending bit is the while loop in dwt::RichTextBox::addTextSteady (dwt/src/widgets/RichTextBox.cpp:258) - that code attempts to remove old lines when adding new ones.

anyone can debug / figure out what precisely the loop does and fix it the right way?

Revision history for this message
poy (poy) wrote :

there were several issues - see rev 87aa1e0fcb086d82243e517310c192a049275582 and the one before for defatils.
essentially, a loop trying to make room for text being added was not finding enough lines to strip off and was therefore running infinitely.
I've also fixed scolling in the process, which I am guessing had never been tested in this particular edge case.

to testers: feel free to play with dwt/test/RichTextBoxTest.cpp - interesting combinations to test these cases:
* REPEATED_TEXT = L"ABC\n"; REPEAT_COUNT = 16384;
* REPEATED_TEXT = L"A\n"; REPEAT_COUNT = 16384;
* REPEATED_TEXT = L"\n"; REPEAT_COUNT = 16384;
* REPEATED_TEXT = L"AAABBB"; REPEAT_COUNT = 16384;

Changed in dcplusplus:
status: Confirmed → Fix Committed
Revision history for this message
eMTee (realprogger) wrote :

I can confirm that the issue is fixed, using the reporter's original test environment.

Revision history for this message
poy (poy) wrote :

Fixed in DC++ 0.866.

Changed in dcplusplus:
status: Fix Committed → Fix Released
eMTee (realprogger)
information type: Private Security → Public
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.