Chat messages are escaped in an unsafe way
Bug #723379 reported by
Nicolai Hähnle
This bug affects 1 person
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
widelands |
Fix Released
|
High
|
Unassigned |
Bug Description
At the moment, '<' is replaced with '{' in chat messages in the NetHost to avoid escaping out of rich-text in chat message displays. There are two things wrong with this:
1. This is potentially insecure, because we rely on the host doing this replacement. A malicious host could trivially disable this replacement and target individual clients with nasty spam in richtext messages.
2. Clearly it would be nicer if players could actually write '<' in their messages.
A much better approach would be to transfer chat messages verbatim, and to perform any necessary escaping in a proper way in ChatMessage:
To post a comment you must log in.
I'm torn as to where this should be targeted. I don't believe this can be used to DoS somebody, let alone execute arbitrary code; then again, those are famous last words when it comes to network security, and I feel really, really uneasy about knowingly releasing code that doesn't sanitize its inputs properly.
I very strongly tend towards moving the < to { substitution to ChatMessage: :toPrintable, where it logically belongs.
Properly fixing the escaping so that we can render < signs also in chat messages could be left to build17, though.