From: Rudolf Polzer <divver...@gmail.com>

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


Reply via email to