Lobby crash when posting special characters

[bb]

Sending code \U0001f35\u274\u2744\U0001f34Merry Xmas \U0001f35\u274\u2744\U0001f35 as pm in multiplayer lobby results in a segfault for the reciever. Found by Hannibal_Baraq

Segfault:

Function call failed: return value was -100701 (Code point outside BMP (> 0x10000))
Location: utf8.cpp:84 (RaiseError)

Call stack:

(0x905f9e) /usr/bin/pyrogenesis() [0x905f9e]
(0x8b1499) /usr/bin/pyrogenesis() [0x8b1499]
(0x8b1865) /usr/bin/pyrogenesis() [0x8b1865]
(0x8b1be8) /usr/bin/pyrogenesis() [0x8b1be8]
(0x8b47b1) /usr/bin/pyrogenesis() [0x8b47b1]
(0x947beb) /usr/bin/pyrogenesis() [0x947beb]
(0x959b0d) /usr/bin/pyrogenesis() [0x959b0d]
(0x7ff50faa2a0d) /lib64/libgloox.so.13(_ZN5gloox7MUCRoom13handleMessageERKNS_7MessageEPNS_14MessageSessionE+0x28d) [0x7ff50faa2a0d]
(0x7ff50fa4cb47) /lib64/libgloox.so.13(_ZN5gloox10ClientBase21notifyMessageHandlersERNS_7MessageE+0x3f7) [0x7ff50fa4cb47]
(0x7ff50fa52f0a) /lib64/libgloox.so.13(_ZN5gloox10ClientBase9handleTagEPNS_3TagE+0x4ea) [0x7ff50fa52f0a]
(0x7ff50fa48662) /lib64/libgloox.so.13(_ZN5gloox6Parser8closeTagEv+0x112) [0x7ff50fa48662]
(0x7ff50fa49678) /lib64/libgloox.so.13(_ZN5gloox6Parser4feedERNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE+0xa28) [0x7ff50fa49678]
(0x7ff50fa4deda) /lib64/libgloox.so.13(_ZN5gloox10ClientBase5parseERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE+0x5a) [0x7ff50fa4deda]
(0x7ff50fa4a32c) /lib64/libgloox.so.13(_ZN5gloox19ConnectionTCPClient4recvEi+0xfc) [0x7ff50fa4a32c]
(0x43022f) /usr/bin/pyrogenesis() [0x43022f]
(0x421927) /usr/bin/pyrogenesis() [0x421927]

errno = 0 (No error reported here)
OS error = ?

[elexis]

Allows to kill the entire lobby if posting it publicly. 🎅❄❄🎄Merry Xmas🎄❄❄🎅 is that specific string.

🎅 and 🎄 trigger that crash, ❄ only prints an unreadable character.

[elexis]

All unicode characters outside of BMP (​Basic_Multilingual_Plane) trigger that assertion.

Here a less obfuscated backtrace:

Thread 1 "pyrogenesis" received signal SIGTRAP, Trace/breakpoint trap.
0x00007ffff349a767 in kill () at ../sysdeps/unix/syscall-template.S:84
84	../sysdeps/unix/syscall-template.S: No such file or directory.
(gdb) bt
#0  0x00007ffff349a767 in kill () at ../sysdeps/unix/syscall-template.S:84
#1  0x0000000000911b98 in debug_break () at ../../../source/lib/sysdep/os/unix/udbg.cpp:50
#2  0x00000000008bbd23 in RaiseError (perr=0x0, err=-100702) at ../../../source/lib/utf8.cpp:84
#3  ReplaceIfInvalid (err=0x0, u=<optimized out>) at ../../../source/lib/utf8.cpp:100
#4  wstring_from_utf8 (src="🎅 \t  \t\t\t\t \t \t \t    \t\t  \t   \t\t  \t\t", err=err@entry=0x0) at ../../../source/lib/utf8.cpp:237
#5  0x000000000095087b in XmppClient::handleMessage (this=0x1d3ba40, msg=...) at ../../../source/lobby/XmppClient.cpp:676
#6  0x0000000000962545 in glooxwrapper::MessageHandlerWrapper::handleMessage (this=0x1c05580, msg=...) at ../../../source/lobby/glooxwrapper/glooxwrapper.cpp:112
#7  0x00007ffff502cd0a in gloox::ClientBase::notifyMessageHandlers(gloox::Message&) () from /usr/lib/x86_64-linux-gnu/libgloox.so.13
#8  0x00007ffff5032cd8 in gloox::ClientBase::handleTag(gloox::Tag*) () from /usr/lib/x86_64-linux-gnu/libgloox.so.13
#9  0x00007ffff502846a in gloox::Parser::closeTag() () from /usr/lib/x86_64-linux-gnu/libgloox.so.13
#10 0x00007ffff50296d8 in gloox::Parser::feed(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) () from /usr/lib/x86_64-linux-gnu/libgloox.so.13
#11 0x00007ffff502defa in gloox::ClientBase::parse(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) () from /usr/lib/x86_64-linux-gnu/libgloox.so.13
#12 0x00007ffff502a35c in gloox::ConnectionTCPClient::recv(int) () from /usr/lib/x86_64-linux-gnu/libgloox.so.13
#13 0x000000000043067f in Frame () at ../../../source/main.cpp:347
#14 RunGameOrAtlas (argc=argc@entry=1, argv=argv@entry=0x7fffffffe028) at ../../../source/main.cpp:530
#15 0x0000000000421d27 in main (argc=1, argv=0x7fffffffe028) at ../../../source/main.cpp:570

The breakpoint is triggered whenever wstring_from_utf8 encounters such a character, so it can occur in other instances as well when receiving user input (Notice that usual chat input elements don't yield such characters in the first place, so it has to be inserted without the GUI of 0 A.D.).

Similar errors are defined at the top of utf8.cpp:

static const StatusDefinition utf8StatusDefinitions[] = {
	{ ERR::UTF8_SURROGATE, L"UTF-16 surrogate pairs aren't supported" },
	{ ERR::UTF8_OUTSIDE_BMP, L"Code point outside BMP (> 0x10000)" },
	{ ERR::UTF8_NONCHARACTER, L"Noncharacter (e.g. WEOF)" },
	{ ERR::UTF8_INVALID_UTF8, L"Invalid UTF-8 sequence" }
};

The attached patch removes the breakpoint and prints the existing human-readable string as a warning.

[elexis]

The debug breakpoint can be considered useful, since the call stack / backtrace reveals which function received that unsupported character first.

Afaik (didn't succeed when trying), if a debugger is attached, one can also lookup the variable contents to find out which string is affected.

The debug breakpoint could become optional using an ifdef.

Alternatively or additionally, the error code could be passed to all those wstring_from_utf8 and related calls and complain in that case (for example "Invalid character in a lobby message").

Ultimately the breakpoint could also stay enabled by default (so that non-developers are also incentivized to report callstacks) but optionally disabled in certain occurances (like the lobby).

[elexis]

This is a very handy moderation tool though and mods should be able to stop players from participating in lobbied games, so we should implement ​https://code.wildfiregames.com/D116 beforehand ideally and the follow-up patch

[elexis]

In 19540:

Remove debug breakpoint that is triggered whenever an unsupported UTF32 character is encountered,
for example when someone would send a lobby chat message containing such a character, it effectively ended the game of the recipients.

Differential Revision: ​https://code.wildfiregames.com/D456
Fixes #4433
Reviewed By: leper
Reported and tested by: Hannibal_Barca