eelcocramer/node-bluetooth-serial-port — pull request #11 by langholz (2013-08-30 23:30:10+00:00)
Title: Fix for buffer overflow on read failure

On linux, whenever a connection is already established and the read operation on the file descriptor is unsuccessful (e.g. the connection was reset or other), the application crashes with a segmentation fault (e.g. Program received signal SIGSEGV, Segmentation fault). Debugging a little bit more through gdb, I discovered the following...

Problem:
In function BTSerialPortBinding::EIO_Read for src/linux/BTSerialPortBinding.cc:163, we assign the number of bytes read by the read(...) to baton->size. However, upon the operation failing the bytes read return is -1. A couple of lines below in the same function, src/linux/BTSerialPortBinding.cc:169, we copy the data read and stored in buf into baton->result. However, in the case the read operation failed baton->size == -1. This causes memcpy to be invoked with a -1 as its count value. The count parameter is actually defined as a size_t while baton->size happens to be an int. Passing the -1 (int) into memcpy causes the count (size_t is actually unsigned) value to wrap around and become a really large value (integer overflow). In turn, when the copy function refers to an index outside either buffers we overflow because we are referring to a memory location larger than 1024 (which is the max size for either).

Solution:
Luckily the solution is simple. We just need to add a check to determine if the size is non-negative. If it is not, then we proceed to copy. Otherwise, we don't. I tried this fix with the application that I used to uncover this issues and confirmed that this works as expected.