Author Topic: Server crashes when MOTD has a double quote?  (Read 397 times)

0 Members and 1 Guest are viewing this topic.

Offline jamespetts

  • Simitrans-Extended project coordinator
  • Devotee
  • *
  • Posts: 15691
  • Total likes: 395
  • Helpful: 174
  • Cake baker
    • Bridgewater-Brunel
  • Languages: EN
Server crashes when MOTD has a double quote?
« on: September 30, 2017, 10:25:00 AM »
I am not entirely sure that this affects the latest version of Standard, as I have not tested this fully (this is a very time consuming issue to test), but the latest version of Extended on the master branch appears to have a bug that causes the server to crash on saving a game when the MOTD contains a double quote (") character.

Because the code for saving the MOTD is unchanged between Standard and Extended and, I believe, unchanged recently, it seems likely that this is also a bug in Standard, although I have not had time to test this.

A workaround is to use single quotes (').
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

Offline Dwachs

  • DevTeam, Coder/patcher
  • Administrator
  • *
  • Posts: 4247
  • Total likes: 175
  • Helpful: 149
  • Languages: EN, DE, AT
Re: Server crashes when MOTD has a double quote?
« Reply #1 on: October 01, 2017, 09:25:16 AM »
The crash happens during saving at the server?

Edit: From looking at the code, I think there is a problem: motd[len]=0 writes out of bounds, as motd has length len.......
Parsley, sage, rosemary, and maggikraut.

Offline jamespetts

  • Simitrans-Extended project coordinator
  • Devotee
  • *
  • Posts: 15691
  • Total likes: 395
  • Helpful: 174
  • Cake baker
    • Bridgewater-Brunel
  • Languages: EN
Re: Server crashes when MOTD has a double quote?
« Reply #2 on: October 01, 2017, 09:31:02 AM »
Yes, I think so - it crashes, then the saved game is corrupted (and always the same file size for any given saved game to start with).
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

Offline Dwachs

  • DevTeam, Coder/patcher
  • Administrator
  • *
  • Posts: 4247
  • Total likes: 175
  • Helpful: 149
  • Languages: EN, DE, AT
Re: Server crashes when MOTD has a double quote?
« Reply #3 on: October 01, 2017, 09:31:53 AM »
From looking at the code, I think there is a problem: motd[len]=0 writes out of bounds, as motd has length len.......

I cannot test right now.
Parsley, sage, rosemary, and maggikraut.

Offline Ters

  • Coder/patcher
  • Devotee
  • *
  • Posts: 4807
  • Total likes: 191
  • Helpful: 108
  • Languages: EN, NO
Re: Server crashes when MOTD has a double quote?
« Reply #4 on: October 01, 2017, 11:49:10 AM »
Where in the code is this motd[len]=0? It certainly looks suspicious, but that depends on what len is supposed to be, string length or buffer size.

Offline Dwachs

  • DevTeam, Coder/patcher
  • Administrator
  • *
  • Posts: 4247
  • Total likes: 175
  • Helpful: 149
  • Languages: EN, DE, AT
Re: Server crashes when MOTD has a double quote?
« Reply #5 on: October 01, 2017, 04:14:10 PM »
Should be fixed in r8298.

James, thanks for reporting.
Parsley, sage, rosemary, and maggikraut.

Offline DrSuperGood

Re: Server crashes when MOTD has a double quote?
« Reply #6 on: October 05, 2017, 11:16:24 AM »
Code: [Select]
char *motd = (char *)malloc( len );Code like this makes Simutrans look like it has an identity crisis. I thought Simutrans was meant to be C++ but that is clearly C code...

I would imagine the C++ form should look like...
Code: [Select]
char *const motd = new char[len];
...
delete[] motd;
An argument for this is that it removes a typecast and explicitly shows that an array is being allocated so is more readable (malloc allocates general memory which may or may not end up being used as an array). Ideally std:string would be used as that automatically manages memory but that requires at least C++11 to be compatible in this use case (earlier C++ versions do not permit the direct modification of c_str).

Offline Ters

  • Coder/patcher
  • Devotee
  • *
  • Posts: 4807
  • Total likes: 191
  • Helpful: 108
  • Languages: EN, NO
Re: Server crashes when MOTD has a double quote?
« Reply #7 on: October 05, 2017, 05:33:42 PM »
earlier C++ versions do not permit the direct modification of c_str

Arguably, no C++ version should permit direct modification of c_str. If safety is the goal, any kind of modification should be disallowed.

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 8795
  • Total likes: 319
  • Helpful: 229
  • Languages: De,EN,JP
Re: Server crashes when MOTD has a double quote?
« Reply #8 on: October 06, 2017, 02:39:42 AM »
I would prefer "char *const motd = new char[len];" It was avoided in the past, because new with zero used to crash MSVC compiled programs, even though it should not (according to C++ standard.)

Offline DrSuperGood

Re: Server crashes when MOTD has a double quote?
« Reply #9 on: October 06, 2017, 03:40:41 AM »
Quote
Arguably, no C++ version should permit direct modification of c_str. If safety is the goal, any kind of modification should be disallowed.
I see where that could be a problem now. When looking up UTF-8 with UTF-16 I read this article which after giving a second read does some pretty unsafe stuff. It takes the address of the first operator[] element in a std::string and uses that as a char array, however I do not think std::string guarantees that each sequential element returned by operator[] is address wise sequential and hence an array despite the operator being array like.
Quote
I would prefer "char *const motd = new char[len];" It was avoided in the past, because new with zero used to crash MSVC compiled programs, even though it should not (according to C++ standard.)
I cannot find an internet reference to that so possibly it was a short lived bug? Not like it could happen in this case as it looks like a minimum value of 1 is enforced.