Position Causes Stockfish and Komodo To Crash

Discussion of chess software programming and technical issues.

Moderators: hgm, Rebel, chrisw

gaard
Posts: 447
Joined: Mon Jun 07, 2010 3:13 am
Location: Holland, MI
Full name: Martin W

Re: Position Causes Stockfish and Komodo To Crash

Post by gaard »

bob wrote: Mon Dec 14, 2020 4:12 am
AndrewGrant wrote: Fri Dec 11, 2020 8:14 am Someone recently wasted their time writing up 20 paragraphs showing how they could "exploit" Stockfish into crashing....

If you send a chess engine garbage, you should expect garbage. If you don't know it is garbage, then maybe a GUI should be doing it for you.

The above has been affirmed many times in Stockfish PRs, where users come saying they have found a "bug"
have to add here... look at any software engineering textbook and find a discussion of "robustness". Hint: bad input should NEVER cause an engine to crash, produce garbage or cause demons to fly out of your nose. Why allow it? Senseless to me.
Ideally, an exception would be thrown. How many GUIs or otherwise would relate such an exception intelligently to the user?
Dann Corbit
Posts: 12540
Joined: Wed Mar 08, 2006 8:57 pm
Location: Redmond, WA USA

Re: Position Causes Stockfish and Komodo To Crash

Post by Dann Corbit »

I don't think a GUI can catch an exception thrown by a program running in its own memory space.
But try/catch is as simple as it gets to catch these kind of things,
And if the 1% speed penalty has one covering in fear, there is always the dreaded if test

I would like for both the GUI and the engine to look for things undefined.
It sounds redundant, but probably not
The GUI has to worry about drawing the picture. It might be very concerned if the side to move field is missing, but not care if there are 10 queens,
The engine (on the other hand) might default to white to move and have an array that can only hold 9 queens.
It may be said that there is no Xboard or UCI API to communicate these errors.
We can write them in a log file, with the offending position and exactly what was wrong, however.
And if nothing else, we could send the information as a kibitz. I think both protocols can handle that
Taking ideas is not a vice, it is a virtue. We have another word for this. It is called learning.
But sharing ideas is an even greater virtue. We have another word for this. It is called teaching.
syzygy
Posts: 5566
Joined: Tue Feb 28, 2012 11:56 pm

Re: Position Causes Stockfish and Komodo To Crash

Post by syzygy »

AndrewGrant wrote: Sun Dec 13, 2020 8:11 pm
Sesse wrote: Sun Dec 13, 2020 12:30 pm
gbtami wrote: Sun Dec 13, 2020 10:56 am Sure. You can substitute current maintainers :wink:
Is there active resistance, or just “nobody has contributed a reasonable patch”?
Typically, a person comes around and says "Look this crashes, fix it!" and everyone says no.
No one, to my knowledge, has actually showed up and presented a solution. So its laziness from the complainers too.
I have seen some proposals (perhaps not full blown PRs) to fix certain cases of crashing on invalid input, but they are not accepted because it doesn't make sense to complicate the code with partial fixes. Much worse than an outspoken "insecure" program is a program that pretends to be "secure" but in reality is not.
syzygy
Posts: 5566
Joined: Tue Feb 28, 2012 11:56 pm

Re: Position Causes Stockfish and Komodo To Crash

Post by syzygy »

Dann Corbit wrote: Mon Dec 14, 2020 1:25 pm I don't think a GUI can catch an exception thrown by a program running in its own memory space.
But try/catch is as simple as it gets to catch these kind of things,
And if the 1% speed penalty has one covering in fear, there is always the dreaded if test
Exceptions are hardly a solution.

A regular program crash is basically the same as throwing an exception. From a security point of view, crashes are harmless. The problems come when the program does not crash but allows a malicious input sequence to run some code, usually via buffer overflows.

Stockfish could be made "secure" by adding a ton of input checks (including lots of tricky logic that UCI was meant to make unnecessary) and making the program exit (or crash) whenever a check fails. It would not decrease performance in any way, except for potentially slowing down engine development.
Dann Corbit
Posts: 12540
Joined: Wed Mar 08, 2006 8:57 pm
Location: Redmond, WA USA

Re: Position Causes Stockfish and Komodo To Crash

Post by Dann Corbit »

syzygy wrote: Mon Dec 14, 2020 3:03 pm
Dann Corbit wrote: Mon Dec 14, 2020 1:25 pm I don't think a GUI can catch an exception thrown by a program running in its own memory space.
But try/catch is as simple as it gets to catch these kind of things,
And if the 1% speed penalty has one covering in fear, there is always the dreaded if test
Exceptions are hardly a solution.

A regular program crash is basically the same as throwing an exception. From a security point of view, crashes are harmless. The problems come when the program does not crash but allows a malicious input sequence to run some code, usually via buffer overflows.

Stockfish could be made "secure" by adding a ton of input checks (including lots of tricky logic that UCI was meant to make unnecessary) and making the program exit (or crash) whenever a check fails. It would not decrease performance in any way, except for potentially slowing down engine development.
A program crash is not harmless. It is one obvious exhibition of undefined behavior (exceeding array bounds, using an uninitialized pointer, etc.) but when undefined behavior happens, any result is possible including exploits. The Morris worm used gets() to write beyond the end of the string, and the data that was injected was program instructions. If undefined behavior exists in a program, it can be deliberately exploited.

Besides which, when Stockfish crashes on my machine, it creates a crash dump. It might sound harmless, but I typically have huge allocations. I also run the tasks in batches. Hence, it is not difficult to write terabytes of crash dumps. It is obvious that this could exhaust virtual memory and make the machine do bad things.

I can change the behavior on my machine so that SF does not write a crash dump when it crashes. But I want this behavior so I can find problems with programs on my machine(s).

So again, you can say it is all my fault.

If people do not want to write robust code, I cannot make it happen. There is an old country expression, "You cannot push a rope."
That is why I stopped making Stockfish error reports. I was pushing a rope.
Taking ideas is not a vice, it is a virtue. We have another word for this. It is called learning.
But sharing ideas is an even greater virtue. We have another word for this. It is called teaching.
syzygy
Posts: 5566
Joined: Tue Feb 28, 2012 11:56 pm

Re: Position Causes Stockfish and Komodo To Crash

Post by syzygy »

Dann Corbit wrote: Mon Dec 14, 2020 6:54 pm
syzygy wrote: Mon Dec 14, 2020 3:03 pm
Dann Corbit wrote: Mon Dec 14, 2020 1:25 pm I don't think a GUI can catch an exception thrown by a program running in its own memory space.
But try/catch is as simple as it gets to catch these kind of things,
And if the 1% speed penalty has one covering in fear, there is always the dreaded if test
Exceptions are hardly a solution.

A regular program crash is basically the same as throwing an exception. From a security point of view, crashes are harmless. The problems come when the program does not crash but allows a malicious input sequence to run some code, usually via buffer overflows.

Stockfish could be made "secure" by adding a ton of input checks (including lots of tricky logic that UCI was meant to make unnecessary) and making the program exit (or crash) whenever a check fails. It would not decrease performance in any way, except for potentially slowing down engine development.
A program crash is not harmless. It is one obvious exhibition of undefined behavior (exceeding array bounds, using an uninitialized pointer, etc.) but when undefined behavior happens, any result is possible including exploits. The Morris worm used gets() to write beyond the end of the string, and the data that was injected was program instructions. If undefined behavior exists in a program, it can be deliberately exploited.
My point is not that undefined behaviour is harmless. My point is that undefined behaviour is harmless when it results in a crash. Many buffer overflows don't result in a crash/won't throw an exception, at least when we're talking C or C++.
So again, you can say it is all my fault.
I think you mentioned that you are now filtering the random input you downloaded from the internet before feeding it to Stockfish. If that is what you do, then I congratulate you for making your script robust.
Ras
Posts: 2487
Joined: Tue Aug 30, 2016 8:19 pm
Full name: Rasmus Althoff

Re: Position Causes Stockfish and Komodo To Crash

Post by Ras »

syzygy wrote: Mon Dec 14, 2020 11:08 pmMy point is that undefined behaviour is harmless when it results in a crash.
The counterpoint is that a "harmless" crash often is only the starting point from where attackers can go and craft an actual attack with carefully chosen payload.
Rasmus Althoff
https://www.ct800.net
Dann Corbit
Posts: 12540
Joined: Wed Mar 08, 2006 8:57 pm
Location: Redmond, WA USA

Re: Position Causes Stockfish and Komodo To Crash

Post by Dann Corbit »

Ras wrote: Mon Dec 14, 2020 11:40 pm
syzygy wrote: Mon Dec 14, 2020 11:08 pmMy point is that undefined behaviour is harmless when it results in a crash.
The counterpoint is that a "harmless" crash often is only the starting point from where attackers can go and craft an actual attack with carefully chosen payload.
Let's consider an uninitialized pointer.
If my memory allocation is small, chances are very, very good that the random address is not in my address space and a fault will occur.
But if I allocate a terabyte of RAM, the probability of the random address being in my space are much higher.
And even if I allocate only 12 bytes, by pure random chance "one in lighting strike" addresses will be in my memory space.
Hence Dr. Hyatt's reference to the 1992 comp.std.c post about nasal demons. (I could have sworn it was Dan Popp and not John Woods, but that was a long time ago).
Taking ideas is not a vice, it is a virtue. We have another word for this. It is called learning.
But sharing ideas is an even greater virtue. We have another word for this. It is called teaching.
Dann Corbit
Posts: 12540
Joined: Wed Mar 08, 2006 8:57 pm
Location: Redmond, WA USA

Re: Position Causes Stockfish and Komodo To Crash

Post by Dann Corbit »

RE: "My point is not that undefined behaviour is harmless. My point is that undefined behaviour is harmless when it results in a crash. Many buffer overflows don't result in a crash/won't throw an exception, at least when we're talking C or C++."

Let's suppose that 100% of the undefined behavior manifests itself as crashes.
Is it harmless when the disk fills with core dumps and virtual memory allocation calls fail?
What if it happens on a disk where you pay for space (say some cloud server)?

I agree that try/catch is a lazy solution and said so myself upthread.
In order to write a really robust solution, a formal grammar for EPD and a parser would be a good start.
But some simple logic like checking the bounds and validity on input likely collected from users (like EPD positions) is a good start.
Taking ideas is not a vice, it is a virtue. We have another word for this. It is called learning.
But sharing ideas is an even greater virtue. We have another word for this. It is called teaching.
syzygy
Posts: 5566
Joined: Tue Feb 28, 2012 11:56 pm

Re: Position Causes Stockfish and Komodo To Crash

Post by syzygy »

Ras wrote: Mon Dec 14, 2020 11:40 pm
syzygy wrote: Mon Dec 14, 2020 11:08 pmMy point is that undefined behaviour is harmless when it results in a crash.
The counterpoint is that a "harmless" crash often is only the starting point from where attackers can go and craft an actual attack with carefully chosen payload.
But then you're still advocating for a false sense of security.

If you don't see any crashes after the program has been patched up enough, you shouldn't assume the program is "secure". The crashes themselves are not the problem.

A typical UCI engine can do funny stuff on malicious input. You have to accept that and not give it input you cannot trust.