Adding semi-colon after syzygy path would crash asmFishW_2017-03-30_popcnt.exe

[fsmosca]

Example path that would crash the engine.
c:\chess\syzygy;J:\Syzygy;G:\Syzygy;

This was mentioned by Dann in CCC and I tried it on asmFishW_2017-03-30_popcnt.exe.

[lantonov]

I think the cause for crashing is clear unlike that for multiple paths.
asmFish expects a single path as was in the original syzygy implementation. When the engine finds ';' in the path, it doesn't know what to do with it as this symbol is forbidden in paths. So the engine stalls.
In Marco's rewrite of the original syzygy code, there is a piece that allows multiple syzygy paths, separated by a semi-colon (on Windows) or colon (in Linux):

public:
    // Look for and open the file among the Paths directories where the .rtbw
    // and .rtbz files can be found. Multiple directories are separated by ";"
    // on Windows and by ":" on Unix-based operating systems.
    //
    // Example:
    // C:\tb\wdl345;C:\tb\wdl6;D:\tb\dtz345;D:\tb\dtz6
    static std::string Paths;

    TBFile(const std::string& f) {

#ifndef _WIN32
        const char SepChar = ':';
#else
        const char SepChar = ';';
#endif
        std::stringstream ss(Paths);
        std::string path;

        while (std::getline(ss, path, SepChar)) {
            fname = path + "/" + f;
            std::ifstream::open(fname);
            if (is_open())
                return;
        }

[fsmosca]

When the engine finds ';' in the path, it doesn't know what to do with it as this symbol is forbidden in paths.

When I tried a path with ';' it works fine, it found the TB's.

id name asmFishW_2017-03-30_popcnt

[...]

setoption name syzygypath value C:\chess\syzygy;C:\chess\syzygy1

info string found 145 tablebases

However adding additional ';' at the end,
setoption name syzygypath value C:\chess\syzygy;C:\chess\syzygy1;
and the engine would crash.

[hero2017]

Just out of curiosity. Is it possible the TB's it found are just from the 1st path (C:\chess\syzygy) and not both?

…

________________________________ From: fsmosca <notifications@github.com> Sent: Friday, March 31, 2017 9:02:22 AM To: lantonov/asmFish Cc: Subscribed Subject: Re: [lantonov/asmFish] Adding semi-colon after syzygy path would crash asmFishW_2017-03-30_popcnt.exe (#42) When the engine finds ';' in the path, it doesn't know what to do with it as this symbol is forbidden in paths. When I tried a path with ';' it works fine, it found the TB's. id name asmFishW_2017-03-30_popcnt [...] setoption name syzygypath value C:\chess\syzygy;C:\chess\syzygy1 info string found 145 tablebases — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub<#42 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ATYgIyQ1xlzUbfMrfaDAJHmHKVBBPc8Kks5rrPlegaJpZM4MvHD0>.

[fsmosca]

In my example, the second path is just a dummy, only the first path has the TB's.

[hero2017]

That may be but it'd be good to confirm that when we use a path like you have which is also what I use except I have three paths in that string separated by a ";" then I wonder if all 3 paths are actually being used. I mean it's possible that we think TB's are working but perhaps only one of those paths is and not the rest. I realize this is a separate issue (if there is one) than the ; crash bug but it made me think and wonder if all paths in the Syzygy Path string are working as expected.

…

________________________________ From: fsmosca <notifications@github.com> Sent: Friday, March 31, 2017 9:32:29 AM To: lantonov/asmFish Cc: hero2017; Comment Subject: Re: [lantonov/asmFish] Adding semi-colon after syzygy path would crash asmFishW_2017-03-30_popcnt.exe (#42) In my example, the second path is just a dummy, only the first path has the TB's. — You are receiving this because you commented. Reply to this email directly, view it on GitHub<#42 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ATYgIwqjubhn81a-dSwif_SSVQLKQ6YXks5rrQBtgaJpZM4MvHD0>.

[fsmosca]

I think this is not difficult to verify if the path is being considered or not.

[tthsqe12]

I will look at this in a day. Obviously, the code that needs to be checked out is around
https://github.com/lantonov/asmFish/blob/master/asmFish/guts/TablebaseCore.asm#L1938

[lantonov]

SEP_CHAR is semi-colon in Windows and colon in Linux according to this piece of asmFish code:

match ='W', VERSION_OS {
SEP_CHAR equ ';'
}

match ='L', VERSION_OS {
SEP_CHAR equ ':'

}

Obviously, I have been wrong to think that asmFish doesn't have code for multiple folders. TablebaseCore.asm is the only code that is not created anew but from disassembly. I'm sure that @tthsqe12 will reach to the bottom of it, however.

[lantonov]

Confirming the above:

setoption name syzygypath value C:\Winboard\Syzygy
info string found 145 tablebases
setoption name syzygypath value C:\Winboard\Syzygy\
info string found 145 tablebases
setoption name syzygypath value "C:\Winboard\Syzygy"
info string found 0 tablebases
setoption name syzygypath value C:\;C:\Winboard\Syzygy
info string found 145 tablebases
setoption name syzygypath value C:\Winboard\Syzygy;

The last command crashes the engine. However, tablebases in multiple paths are recognized, e.g. with C:;C:\Winboard\Syzygy the tablebases are in the second path.

[fsmosca]

I am surprised by this.

setoption name syzygypath value "C:\Winboard\Syzygy"
info string found 0 tablebases

[lantonov]

So am I

[lantonov]

Some more (right slash). The last crashes

setoption name syzygypath value C:/Winboard/Syzygy
info string found 145 tablebases
setoption name syzygypath value C:/Winboard/Syzygy/
info string found 145 tablebases
setoption name syzygypath value "C:/Winboard/Syzygy/"
info string found 0 tablebases
setoption name syzygypath value "C:/Winboard/Syzygy;"
info string found 0 tablebases
setoption name syzygypath value C:/Winboard/Syzygy;C:/
info string found 145 tablebases
setoption name syzygypath value C:/Winboard/Syzygy;C:
info string found 145 tablebases
setoption name syzygypath value C:/Winboard/Syzygy;C::
info string found 145 tablebases
setoption name syzygypath value C:/Winboard/Syzygy/:
info string found 0 tablebases
setoption name syzygypath value C:/Winboard/Syzygy:
info string found 0 tablebases
setoption name syzygypath value C:/Winboard/Syzygy;C:;

[tthsqe12]

alright, the issue is with
https://github.com/syzygy1/Cfish/blob/master/src/tbcore.c#L306
if you give it something like
path1;path2;
the code is going to start searching for a path after the last SEP_CHAR. In this case, there is not one and so the code will start reading past the buffer,

Verdict: both asmFish and Cfish suffer from a buffer overflow with
setoption name syzygypath value path1;path2;
Cfish uses the std malloc which is almost guaranteed to have non-zero bytes past the allocated space. In this case L306 stops at this point.
However, asmFish has a malloc that will almost certainly have zero bytes past the allocated space. In this case, L306 loops until it falls off that page, at which point it will almost certainly seg fault.

a command such as
setoption name syzygypath value path1;path2;
is not 'legal'. The parsing in tbcore is very simple and expects one SEP_CHAR between the paths.

I could try to fool-proof the parsing of syzygypath.
At least it could print the number of tbs found in each path instead of just the total number of found tbs.

Let me know what you all think.

[lantonov]

It's not a big issue. If one wants to crash the engine, he can do it in a million ways.
If fool-proofing the parsing doesn't require a lot of work on your part, then do it.
Otherwise, leave it so.

[hero2017]

Probably easiest solution and it makes sense if I understood you. Simply not allowing us to give it a syzygy path that ends in ";" should do the trick although I never enter the path with a ; and so it was never a problem for me. Like lantonov said, if it's not a lot of work sure otherwise I wouldn't bother.

The total number of TB's for each path in the string is a nice touch too...it'd be even better if we knew whether the total # it gives us for each path is correct but that may require too much effort. But this way we'd know if one of the TB files is perhaps corrupted or not.