search

EionRobb / purple-rocketchat

Rocket.Chat Plugin for libpurple
GNU General Public License v3.0
21 stars 2 forks source link

Pidgin crashes when somebody use "Start a Discussion" (e.g. from webapp) #16

Closed Mi-La closed 3 years ago

Mi-La commented 3 years ago

Using latest master, the plugin causes whole Pidgin crash since somebody use "Start a Discussion". Please see the attached backtrace.

Thread 1 "pidgin" received signal SIGSEGV, Segmentation fault.
__strlen_sse2 () at ../sysdeps/x86_64/multiarch/../strlen.S:120
120 ../sysdeps/x86_64/multiarch/../strlen.S: No such file or directory.

pidgin-backtrace.log

When running the pidgin via gdb, it seems that a new window for the discussion is created with some random name - e.g. "RvwHoPGXuTbA3dxsj" .... but than the SIGSEGV appears.

Mi-La commented 3 years ago

I think I've the problem. In librocketchat.c:1380, there is a call to rc_markdown_to_html and the msg_text argument is NULL. The rc_markdown_to_html uses strlen on that NULL argument here: librocketchat.c:388.

Note that strlen doesn't check for NULL and causes SEGFAULT. I did a quick hack to prevent crash by changing the line 388 from: markdown_len = mkd_line((char *)markdown, strlen(markdown), &markdown_str, flags); to: markdown_len = mkd_line((char *)markdown, markdown != NULL ? strlen(markdown) : 0, &markdown_str, flags);

At least it doesn't crash anymore and the message displays correctly in a new "room-like" window.

Mi-La commented 3 years ago

Should I create a pull request with the one-liner hot fix? :-).

EionRobb commented 3 years ago

Sorry, received your bug report while I was on holiday and put it on the backburner

Rather than doing stuff inline, would it be better to return NULL from rc_markdown_to_html if the input is also NULL so that it doesn't need to go through the libdiscount calls unnecessairly?

Mi-La commented 3 years ago

If you have time to do it properly, please just fix it. I don't need to have my own pull request merged ;-). I was happy enough to temporarily fix the crash and report the issue. FYI: There are more problems with rooms, e.g. after pidgin restart, I don't get new messages from "rooms"/discussions (even though I have auto join and persistent options checked) . But sometimes the messages suddenly arrive, I think that it happens when some user who is in the room changes his status.

EionRobb commented 3 years ago

Fair enough, wasn't about who gets credit for what, but you're more able to recreate the issue so more able to confirm a fix :)

Can you try with the above commit and see if that helps?

Mi-La commented 3 years ago

Fix verified, works well. Thanks.

I'll try to investigate the FYI part of my last comment when I have more time.

EionRobb commented 3 years ago

Awesome, thanks for confirming :)

Appreciate the issue reports :)