Win32XP. Branch: 0.A Master. SIGSEGV. (0x004352a3 acces violation). Catacharset.cpp::UTF8_getch

[WareXD]

OS:
Windows XP, SP3.
Checked:
0.A Branch: Master. Also seems error in latest experimental SDL (cataclysmdda-0.A-2007)

Desc:
Sometime game crashed when trying to select any recipe in Craft Window (simply moving text cursor). Also crashed when rapidly open(key: "*") and close(key:"ESC") Construction window.
Frequent problem. It is not clear what causes it. Occurs spontaneously and only in Craft/Construction windows.
P.S. Sorry, but I'm not C++/C# programmer and not looking good in English. Sorry for that.
But I'll try to help a little for a good game.

Debugger Code::Blocks 13.12:

Program received signal SIGSEGV, Segmentation fault.
At E:\CodeBlock\Cataclysm-DDA-master\src\catacharset.cpp:19
Continuing...
[Inferior 1 (process 2448) exited with code 030000000005]
Debugger finished with status 0

Call stack:

#0 00457974 UTF8_getch(src=0xacde6ec, srclen=0xacde6e8) (E:\CodeBlock\Cataclysm-DDA-master\src\catacharset.cpp:19)
#1 00457E69 cursorx_to_position(line=0xe893fd4 "dart", cursorx=77, prevpos=0x0, maxlen=-1) (E:\CodeBlock\Cataclysm-DDA-master\src\catacharset.cpp:158)
#2 004582F3 utf8_truncate(s=..., length=77) (E:\CodeBlock\Cataclysm-DDA-master\src\catacharset.cpp:262)
#3 004891F2 game::select_crafting_recipe(this=0xcce0048) (E:\CodeBlock\Cataclysm-DDA-master\src\crafting.cpp:630)
#4 0048695F game::craft(this=0xcce0048) (E:\CodeBlock\Cataclysm-DDA-master\src\crafting.cpp:346)
#5 00536046 game::handle_action(this=0xcce0048) (E:\CodeBlock\Cataclysm-DDA-master\src\game.cpp:3430)
#6 00522109 game::do_turn(this=0xcce0048) (E:\CodeBlock\Cataclysm-DDA-master\src\game.cpp:1376)
#7 0070F3F0 main(argc=0, argv=0x322584) (E:\CodeBlock\Cataclysm-DDA-master\src\main.cpp:294)

catacharset.cpp:

8 unsigned UTF8_getch(const char **src, int *srclen)
9 {
10    const unsigned char *p = *(const unsigned char **)src;
11    int left = 0;
12    bool overlong = false;
13    bool underflow = false;
14    unsigned ch = UNKNOWN_UNICODE;
15
16    if (*srclen == 0) {
17        return UNKNOWN_UNICODE;
18    }
19>   if (p[0] >= 0xFC) {

I guess, but I can be wrong:
8 unsigned UTF8_getch(const char **src, int *srclen)
10 const unsigned char *p = *(const unsigned char **)src;
Trouble looking at the absence of an error handler and validating for the string array pointer p and deeper. There is no validation for external src before equate to not init p.
16 if (*srclen == 0) { return UNKNOWN_UNICODE; }
Is not sufficient to check only lenght of string when also working with its pointer. Pointer src can be wrong due error or multithreaded operations and calls and pointer p init/get "random" from stack. Especially when working with global/external variables/constants without declaring, initialization and releasing local variables.

[WareXD]

Meanwhile... I'm trying RTFM about to C++ pointers for noob's... Shit shit shit oh my brains. HolyLoL.))

[KA101]

Well, it's more info about the crafting crash than we've had in a while, so thanks for the report! :-)

[WareXD]

Welcome)) Thanks you for responding! But please wait. Had in a while it's mean that it was fixed?))
If it's not maybe this information will be useful:

FIX look like:
Branch: Master. Latest commit 0fc901c.
catacharset.cpp:

150 int cursorx_to_position(const char *line, int cursorx, int *prevpos, int maxlen)
151 {
FIX>   int utf8strlen = utf8_width(line)-1;
152    int dummy;
153    int i = 0, c = 0, *p = prevpos ? prevpos : &dummy;
154    *p = 0;
155    while(c < cursorx) {
156        const char *utf8str = line + i;
157        int len = ANY_LENGTH;
158        unsigned ch = UTF8_getch(&utf8str, &len);
159        int cw = mk_wcwidth(ch);
160        len = ANY_LENGTH - len;
161
162        if( len <= 0 ) {
163            len = 1;
164        }
165        if(maxlen >= 0 && maxlen < (i + len)) {
166            break;
167        }
168        i += len;
FIX>       if (utf8strlen<i) {break;}
169        if( cw <= 0 ) {
170            cw = 1;
171        }
172        c += cw;
173        if( c <= cursorx ) {
174           *p = i;
175        }
176    }
177    return i;
178}

Inner loop are not breaking when line+i >= lenght of utf8str. And after EOL it will count till cursorx. Function UTF8_getch try to use pointer of utf8str beyond EOL and sometime cause access violation.
Access violation has gone. But i'm not sure are this changes will be correct because i'm not good to know C++ syntax. And how this will affect the rest of code. Need your authority decision.
Thank you.

[KA101]

Initial testing seems to work and not break anything. If you can file a PR, please do so. Otherwise, I can file one on your behalf but will need an e-mail address to do so--Git doesn't want to give you credit based solely on your callsign. A forum PM or IRC message would work; I'm usually on US Eastern evenings, 5-10 PM or so.

[WareXD]

Mmm.. it's strange. When i debuging calls from cursorx_to_position to UTF8_getch in watch list i see clear that string utf8str buffer overflow.
Sorry, I don't understand what is this PR and what i must to do :) But if you want to my email: 'deleted'. I'm from Russia. And if it does not scare you:) i'll try a little bit to help.

[KA101]

We've got an active Russian community on the forum. You might stop in: http://smf.cataclysmdda.com/index.php?board=11.0

[WareXD]

Russian community isn't developers community for this game, so as you wish I'll try to stop there:) Thank you :)

[KA101]

Feel free to stick around. HuXTUS and Reaper are prominent Russians here, as is illi-kun. Probably a few others I'm forgetting. :-/

Shoot me the e-mail in a forum message or something, if you like. Was tied up and didn't get the chance to get the commit attributed.

[WareXD]

Well, I don't like shooting but i'll try :P Because this bug too serious :(
Thanks you for community information and your responding.
Forum message is sent.

[WareXD]

👍 Thank you all very much!