From: Rudolf Polzer <[email protected]>
If more input is read from the engine than there is space in
`engineinputbuf`, these overflows could happen due to use of unlimited
`strcat` and a mistake in use of `strlen` to terminate the string:
- engine provided >= `BUF_SIZE` bytes: `strcat` call would read arbitrarily
beyond `engineinputaux` and write beyond `engineinputbuf`. Then, a NUL
byte will be written `BUF_SIZE` behind the engine input buffer,
usually pointing into arbitrary RAM. In my build, this would clear the
beginning of `answerFromEngineExpected`, which _potentially_ (didn't
happen yet) could cause an endless loop; however order of static
globals is not guaranteed and may depend on compiler. It would also
write nonsense into `zerochar` which could break other calls in funny
ways.
- Engine provided less than `BUF_SIZE` bytes, but more than fit into
`engineinputbuf`: then `strcat` will overflow into whatever follows
`engineinputbuf` (in my build, `zerochar`). Also, a zero write is
written beyond that too, but still into `zerochar`.
- Engine provided more than `BUF_SIZE / 2` bytes: then the final NUL
byte is still written somehwere beyond `engineinputbuf`, in my build
into `zerochar` which admittedly is harmless.
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 b79ed67..f39814e 100644
--- a/src/frontend/engine.cc
+++ b/src/frontend/engine.cc
@@ -148,11 +148,16 @@ int ReadFromEngine( void )
printf( "Error reading engine input.\n" );
} else if ( engineinputready > 0 ) {
/* There are some data from the engine. Store it in buffer */
- strncpy( engineinputaux, zerochar, BUF_SIZE );
nread = read( pipefd_a2f[0], engineinputaux, BUF_SIZE );
+ int prev_len = strlen(engineinputbuf);
+ int nremaining = sizeof(engineinputbuf) - prev_len - 1;
+ if (nread > nremaining) {
+ printf( "Overflow reading engine input. Some input has been cut off.\n"
);
+ nread = nremaining;
+ }
/*write( STDOUT_FILENO, engineinputaux, BUF_SIZE );*/
- strcat( engineinputbuf, engineinputaux );
- engineinputbuf[strlen( engineinputbuf ) + nread] = '\0';
+ memcpy(engineinputbuf + prev_len, engineinputaux, nread);
+ engineinputbuf[prev_len + nread] = '\0';
}
return ( engineinputready );
--
2.39.5
