From: Rudolf Polzer <[email protected]>
Fixes read underflow at `mvstr[-1]` when the move string is empty:
```
$ CXXFLAGS='-O1 -g3 -fsanitize=address' LDFLAGS='-g3 -fsanitize=address'
./configure
$ make -j4
$ src/gnuchess
GNU Chess
Can't open file "(null)": Bad address - using defaults
White (1) :
```
Here, just hit Enter:
```
=================================================================
==2615519==ERROR: AddressSanitizer: stack-buffer-underflow on address
0x7fed51b0011f at pc 0x55ecf95ce8ac bp 0x7ffe8f10ac20 sp 0x7ffe8f10ac18
READ of size 1 at 0x7fed51b0011f thread T0
#0 0x55ecf95ce8ab in ValidateMove(char*, char*)
/home/rpolzer/src/gnuchess/src/frontend/move.cc:567
#1 0x55ecf95b945a in parse_input()
/home/rpolzer/src/gnuchess/src/frontend/cmd.cc:1337
#2 0x55ecf95d608d in NextUserCmd()
/home/rpolzer/src/gnuchess/src/frontend/engine.cc:267
#3 0x55ecf95b4694 in main /home/rpolzer/src/gnuchess/src/main.cc:503
#4 0x7fed53a33ca7 in __libc_start_call_main
../sysdeps/nptl/libc_start_call_main.h:58
#5 0x7fed53a33d64 in __libc_start_main_impl ../csu/libc-start.c:360
#6 0x55ecf95b38e0 in _start
(/home/rpolzer/homedir/src/gnuchess/src/gnuchess+0x278e0) (BuildId:
b1b59d825678eda5d98b232e262b6913e2b55ec4)
Address 0x7fed51b0011f is located in stack of thread T0 at offset 31 in frame
#0 0x55ecf95ce72c in ValidateMove(char*, char*)
/home/rpolzer/src/gnuchess/src/frontend/move.cc:537
This frame has 1 object(s):
[32, 160) 'mvstr' (line 541) <== Memory access at offset 31 underflows this
variable
HINT: this may be a false positive if your program uses some custom stack
unwind mechanism, swapcontext or vfork
(longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-underflow
/home/rpolzer/src/gnuchess/src/frontend/move.cc:567 in ValidateMove(char*,
char*)
Shadow bytes around the buggy address:
0x7fed51affe80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x7fed51afff00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x7fed51afff80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x7fed51b00000: f1 f1 f1 f1 00 00 00 00 00 00 00 00 00 00 00 00
0x7fed51b00080: 00 00 00 00 f3 f3 f3 f3 00 00 00 00 00 00 00 00
=>0x7fed51b00100: f1 f1 f1[f1]00 00 00 00 00 00 00 00 00 00 00 00
0x7fed51b00180: 00 00 00 00 f3 f3 f3 f3 00 00 00 00 00 00 00 00
0x7fed51b00200: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x7fed51b00280: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x7fed51b00300: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x7fed51b00380: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
==2615519==ABORTING
```
Also, could remove removal of +#= suffixes - the very thing crashing -
by simply already removing the characters in the preceding copy loop
(was already the case for all except #). Hope that's OK - the main
difference is that now `#e2e4` is read as a valid move, and not as an
error. If that is not OK and `#` was intended as a comment character,
can revise (should then also skip the line and not show "Invalid
move: #e2e4", though).
---
src/frontend/move.cc | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/frontend/move.cc b/src/frontend/move.cc
index 3163166..6904e08 100644
--- a/src/frontend/move.cc
+++ b/src/frontend/move.cc
@@ -559,13 +559,10 @@ leaf * ValidateMove (char *s, char *cleanMove)
p = mvstr;
do
{
- if (*s != 'x' && *s != '+' && *s != '=' && !isspace(*s))
+ if (*s != 'x' && *s != '+' && *s != '#' && *s != '=' && !isspace(*s))
*p++ = *s;
} while (*s++ != '\0' );
- /* Flush castles that check */
- if (mvstr[strlen(mvstr)-1] == '+' || mvstr[strlen(mvstr)-1] == '#' ||
- mvstr[strlen(mvstr)-1] == '=') mvstr[strlen(mvstr)-1] = '\000';
if (cleanMove) strcpy(cleanMove, mvstr);
/* Check for castling */
--
2.39.5
