From: Rudolf Polzer <[email protected]>
If more input is read from the user than there is space in
`userinputbuf`, these overflows could happen due to use of unlimited
`strcat` and a mistake in use of `strlen` to terminate the string.
However, as `get_line` is limited to 128 bytes, this can only actually
happen if for some reason `userinputbuf` is not fetched frequently
enough. The trick to show corruption, thus, is to use the fact
that NextUserCmd runs only once per ReadFromUser call, and thus, by
sending a long string of e.g. `"e4\n"`, one may be able to crash:
```
$ CXXFLAGS='-O1 -g -fsanitize=address' LDFLAGS='-g -fsanitize=address'
./configure
$ make clean
$ make -j4
$ perl -e 'syswrite STDOUT, "e4\n" x 5000' | src/gnuchess
[...]
==195822==ERROR: AddressSanitizer: global-buffer-overflow on address
0x564c469d4c40 at pc 0x564c468b2aa3 bp 0x7ffebdac90b0 sp 0x7ffebdac90a8
WRITE of size 1 at 0x564c469d4c40 thread T0
#0 0x564c468b2aa2 in ReadFromUser()
/home/rpolzer/src/gnuchess/src/frontend/engine.cc:194
#1 0x564c468a6116 in main /home/rpolzer/src/gnuchess/src/main.cc:512
#2 0x7f554dc0fca7 in __libc_start_call_main
../sysdeps/nptl/libc_start_call_main.h:58
#3 0x7f554dc0fd64 in __libc_start_main_impl ../csu/libc-start.c:360
#4 0x564c468a58a0 in _start
(/home/rpolzer/homedir/src/gnuchess/src/gnuchess+0xb8a0) (BuildId:
dafa29121d472027c59df619d24db9a46083b3a3)
0x564c469d4c40 is located 32 bytes before global variable 'zerochar' defined in
'engine.cc:52:13' (0x564c469d4c60) of size 4096
0x564c469d4c40 is located 0 bytes after global variable 'userinputbuf' defined
in 'engine.cc:55:6' (0x564c469d3c40) of size 4096
SUMMARY: AddressSanitizer: global-buffer-overflow
/home/rpolzer/src/gnuchess/src/frontend/engine.cc:194 in ReadFromUser()
```
The fix is to redo the buffer management in a working way.
---
src/frontend/engine.cc | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/src/frontend/engine.cc b/src/frontend/engine.cc
index f39814e..85e15fb 100644
--- a/src/frontend/engine.cc
+++ b/src/frontend/engine.cc
@@ -188,10 +188,15 @@ void ReadFromUser( void )
printf( "Error reading user input.\n" );
} else if ( userinputready > 0 ) {
/* There are some data from the user. Store it in buffer */
- strncpy( userinputaux, zerochar, BUF_SIZE );
nread = read( pipefd_i2f[0], userinputaux, BUF_SIZE );
- strcat( userinputbuf, userinputaux );
- userinputbuf[strlen( userinputbuf ) + nread] = '\0';
+ int prev_len = strlen(userinputbuf);
+ int nremaining = sizeof(userinputbuf) - prev_len - 1;
+ if (nread > nremaining) {
+ printf( "Overflow reading user input. Some input has been cut off.\n" );
+ nread = nremaining;
+ }
+ memcpy(userinputbuf + prev_len, userinputaux, nread);
+ userinputbuf[prev_len + nread] = '\0';
}
}
--
2.39.5
