Bad NMDC validation messes up LinuxDC++ chat formatting

Bug #541548 reported by Razzloss on 2010-03-18
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
DC++
Undecided
Unassigned
LinuxDC++
Medium
Razzloss

Bug Description

Seen at least on one hub out in the wild, which terminated actions/emotes in chat with \0 instead of proper |. E.g. Hub commands sent with '+me is going to lose his mind with this' were sent out by hub as * razzloss is going to his mind with this\0 The ClientListener Status message fired from this then contained this line and the next commands send by the hub until valid termination character was found.

Windows clients apparently behaved as everything went fine (since no-one hadn't bothered to fix the problem and they saw the messages). So I'd imagine Windows chat messages are strcpy'd at some point, so that they end at correct place (and won't output the following commands to chat).

LinuxDC++ on the otherhand passes the length of the string from the fired StatusMessage to gtk_text_buffer_insert, which then spews bunch of utf8 validation errors to console and doesn't actually add anything to the chat. After this nick tags are applied by LinuxDC++ so random text on the previous line gets italicized.

So how do we fix this? At minimum the italicizing should be corrected, but should some validation be added to dcpp, so that messages like this won't get up to the GUI level?

Quick fix which will show the emote line correctly is to change the len parameter passed to gtk_text_buffer_insert to -1, so that it will use only the null terminated part of the message.

edit: DC++ core version used was 0.75 (also happened with released linuxdcpp with 0.698 core)

Related branches

Razzloss (razzloss) wrote :
  • hub Edit (583 bytes, text/x-python)

This behavior can be seen with the attached 'hub'. Replace the razzloss in 9th line with the nick used by your client.

Razzloss (razzloss) on 2010-03-18
description: updated
Steven Sheehy (steven-sheehy) wrote :

I think setting gtk_text_buffer_insert to -1 should be fine.

As far as the core, shouldn't it be made to handle the scenario where it encounters a dollar sign that's not at the beginning of the command? As I understand it, if the client wanted to send a '$' for display to the user it should come as a HTML escape sequence, right? In that case, we can simply drop the invalid NMDC command and continue with the second command. For example, if we received:

"* chim says meh\0$Search 8.8.5.1:1412 F?T?0?9?TTH:XXXXXXXE7RFZ2CATWVCFSF3A4UFHHJDO5HXZRFI|"

NmdcHub:onLine(line) would reject the string "* chim says meh\0", trim it from the line, and restart processing using "Search 8.8.5.1:1412 F?T?0?9?TTH:XXXXXXXE7RFZ2CATWVCFSF3A4UFHHJDO5HXZRFI|" as the new string. It may also might be helpful to reject '|' if it's not at the end of the string. I admit I'm not familiar enough with the NMDC protocol to know if this would cause any issues. DC++ team, thoughts?

Razzloss (razzloss) wrote :

LinuxDC++ GUI now drops the data after \0 before attempting to insert it in chat buffer.

--RZ

Changed in linuxdcpp:
assignee: nobody → Razzloss (razzloss)
milestone: none → 1.1.0
status: Confirmed → Fix Committed
tags: added: ui
removed: gui
Changed in dcplusplus:
status: New → Won't Fix
Changed in linuxdcpp:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers

Bug attachments